Closed
Bug 90956
Opened 23 years ago
Closed 22 years ago
Dot (.) instead of localized characters in pipnss module
Categories
(Core Graveyard :: Security: UI, defect, P4)
Tracking
(Not tracked)
VERIFIED
FIXED
psm2.2
People
(Reporter: m.wawoczny, Assigned: KaiE)
References
Details
(Keywords: intl, l12y, Whiteboard: [adt2 RTM] [eta 6/24])
Attachments
(8 files, 8 obsolete files)
(deleted),
image/gif
|
Details | |
(deleted),
image/gif
|
Details | |
(deleted),
image/gif
|
Details | |
(deleted),
image/gif
|
Details | |
(deleted),
image/gif
|
Details | |
(deleted),
image/gif
|
Details | |
(deleted),
patch
|
KaiE
:
review+
alecf
:
superreview+
jud
:
approval+
|
Details | Diff | Splinter Review |
(deleted),
image/gif
|
Details |
I can see only dot (.) in some dialogs from pipnss module. See atached
screenshoot. Chars are coded in escape-unicode, so this is not problem with
coding (see second screenshoot where localized characters are viewed properly -
this screenshoot is from composer). This happen with .properties files. String
for this dialog is in pipnss.properties - InternalToken/PrivateTokenDescription
property.
Reporter | ||
Comment 1•23 years ago
|
||
Reporter | ||
Comment 2•23 years ago
|
||
Comment 3•23 years ago
|
||
I cannnot find "pipnss.properties" in lxr.mozilla.org.
Could you specify the full path of the file?
Reporter | ||
Comment 4•23 years ago
|
||
http://lxr.mozilla.org/mozilla/source/security/manager/ssl/resources/locale/en-0
and
http://lxr.mozilla.org/mozilla/source/security/manager/ssl/resources/locale/en-1
Or in en-US.jar file in \locale\pipnss\pipnss.properties
Comment 5•23 years ago
|
||
Could you explain what is wrong in the first screen shot?
Is the problem in the window title, buttons, or the alert text?
The alert text actually shows some non ASCII characters.
Keywords: l12y
Reporter | ||
Comment 6•23 years ago
|
||
In first screenshoot we got "Programowe Urz.dzenie Zabezpiecz." instead of
"Programowe Urz±dzenie Zabezpieczaj±ce". Char "±" (escape-unicode code is
\u0105) is viewed as ".". Second "Zabezpieczaj±ce" was trimed to "Zabezpiecz."
"Programowe Urz±dzenie Zabezpieczaj±ce" = english "Software Security Device" =
escape-unicode "Programowe Urz\u0105dzenie Zabezpieczaj\u0105ce"
String is escape-unicode, when it is UTF-8 char also aren't displayed corectly.
Updated•23 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 7•23 years ago
|
||
Thank you for the info.
Change component to security and reassign to mstoltz@netscape.com, cc to l10n
people.
Comment 9•23 years ago
|
||
NSS/PSM issues should be assigned to the PSM product, not the Security:General
component.
Assignee: mstoltz → ssaux
Component: Security: General → Client Library
Product: Browser → PSM
QA Contact: ylong → junruh
Version: other → 2.0
Comment 11•23 years ago
|
||
Moving all P3 and P4 bugs targetted to 2.1 to future.
Target Milestone: 2.1 → Future
Updated•23 years ago
|
Comment 13•23 years ago
|
||
Marking nsbranch- as it was decided in the August bug triage that we wouldn't
have enough time in eMojo to fix this. Let's revisit for MachV.
Keywords: nsbranch-
Comment 14•23 years ago
|
||
removed keyword nsbranch since bug now has nsbranch-, per pdt mtg.
Keywords: nsbranch
Comment 15•23 years ago
|
||
OS > all
OS: Windows 2000 → All
QA Contact: ckritzer → junruh
Hardware: PC → All
Version: 2.0 → 2.1
Comment 16•23 years ago
|
||
ssaux - can you please provide status on this one. It would be nice to have it
fixed for machv. thanks
Comment 17•23 years ago
|
||
The string used for "Software Security Device" must be UTF8 encode as per the
PKCS 11 specs, and the encoded string of byte is limited to 32 bytes. If the
string is less then 32 bytes, the string must be padded with blanks.
The string that fails to display correctly in that language would take more than
32 bytes to store in UTF8 and therefore is not valid.
The string needs to use abbreviations and the translators need to understand
that whatever string they come up with must fit in a 32 byte quantity.
I'm invesigating whether a string of the proper length with "±" will display
correctly.
Reporter | ||
Comment 18•23 years ago
|
||
Seems that other strings connected with Security Devices are screwed up.
See another screenshoot.
> The string used for "Software Security Device" must be UTF8 encode
Why in UTF8 and not unicode escape - this strings are in .properties file which should be unicode escape?
> I'm invesigating whether a string of the proper length with "±" will display
> correctly.
I've checked this with UTF8 and escape unicode and this is not working :(
Build ID 2001111708
Reporter | ||
Comment 19•23 years ago
|
||
Comment 20•23 years ago
|
||
The UTF8 requirement is as per the PKCS11 specs.
We need help from the internationalization group to get a working dev env with
the proper language packs. CVS only stores en-US, and that's just not enough. I
need the ability to regenerate localized jar for the affected language to
reproduce the bug in a dev env.
Comment 21•23 years ago
|
||
UTF8 is the standard chosen by the PKCS #11 working group as a way to
internationalize PKCS #11, which originally specified the strings as ASCII
strings. These strings are passed in fixed arrays of 32 bytes, padded with
spaces (optionally terminated with NULLs).
We can specify the strings anyway we want, but the must be converted to UTF8 and
truncted to 32 bytes somewhere in the code, then returned back as UTF8 before
display. In Communicator these restrictions were even more strictly enforced
(strings passed down to for rejected if the were not exactly 32 bytes long).
This meant that formatting problems with those strings were found very early in
the translation process, and the requirements quickly communicated back. As a
result of one of these, it was requested that we allow strings that do not meet
the exact formating requirements, and reformat the string internally.
I believe we need to provide clear documentation on these properties so
translaters know that must fit their strings in 32 bytes, either by abreviating
some of the words in a meaningful way in that language, or by providing and
alternate, shorter phrase to describe the software security token (other than
direct translation). There are about 8 or so strings that have these
requirements (some limitted to 32 bytes, others limitted to 64 bytes.)
bob
Comment 22•23 years ago
|
||
> We need help from the internationalization group to get a working dev env with
> the proper language packs. CVS only stores en-US, and that's just not enough. I
> need the ability to regenerate localized jar for the affected language to
> reproduce the bug in a dev env.
Any chance we can use the contributed langpacks in
http://www.mozilla.org/projects/l10n/mlp_status.html#contrib for testing purpose?
Comment 23•23 years ago
|
||
Comments from rchen:
For the debugging purpose, you don't need the correct translation which we may
not even have. You can just copy the any strings from any files in the same
encoding which you want to verify. Does bug 90956 happen to Ja strings? Will
our JA files help to verify the bug? Anyhow, I attach JA pipnss.properties in
the email.
The so mentioned property file will be attached.
Comment 24•23 years ago
|
||
OK, here is one already created in another bug
(http://bugzilla.mozilla.org/show_bug.cgi?id=88280):
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=58389.
Comment 25•23 years ago
|
||
How do I install this in the dev env? Do I make a copy of the entire en-US dir?
I'm asking because one of the things I tried was to changed the
en-US/pipnss.properties file by adding the character that fails to show up, but
as a result, Mozilla ends up using a completely different set of strings for the
security device dialog. It appears that while loading the file, something goes
wrong and some default file which I was unable to locate is loaded instead.
I don't want a released package, I want something I can compile in.
Comment 26•23 years ago
|
||
You can just replace the English strings with the Unicode strings for debugging.
You don't need to change the environment. I copy some of them here:
SignedBy=\u7f72\u540d\u8005\uff1a%S
CertPassPrompt=%S \u306e\u30de\u30b9\u30bf\u30fc
\u30d1\u30b9\u30ef\u30fc\u30c9\u3092\u5165\u529b\u3057\u3066\u304f\u3060\u3055\u
3044\u3002
RootCertModuleName=Builtin Roots Module
ManufacturerID=Mozilla.org
LibraryDescription=PSM Internal Crypto Services
TokenDescription=Generic Crypto Services
PrivateTokenDescription=Software Security Device
SlotDescription=PSM Internal Cryptographic Services
PrivateSlotDescription=PSM \u79d8\u5bc6\u30ad\u30fc
FipsSlotDescription=PSM Internal FIPS-140-1 Cryptographic Services
FipsPrivateSlotDescription=PSM FIPS-140-1 User Private Key Services
InternalToken=Software Security Device
VerifySSLClient=SSL \u30af\u30e9\u30a4\u30a2\u30f3\u30c8\u8a3c\u660e\u66f8
VerifySSLServer=SSL \u30b5\u30fc\u30d0\u8a3c\u660e\u66f8
VerifySSLStepUp=SSL
\u30b5\u30fc\u30d0\uff0f\u30b9\u30c6\u30c3\u30d7\u30a2\u30c3\u30d7
VerifySSLCA=SSL \u8a8d\u8a3c\u5c40
VerifyEmailSigner=\u96fb\u5b50\u30e1\u30fc\u30eb\u7f72\u540d\u8005\u8a3c\u660e\u
66f8
Comment 27•23 years ago
|
||
And you need to switch to Japanese font to display them correctly.
Comment 28•23 years ago
|
||
Ok so I updated one string in my locale/en-US/pipnss.properties to read:
PrivateTokenDescription=Software Securit\u0105 Device
Which should display the character. The character is indeed loaded, but the
window shows a little square instead of the expected character. I must not have
the fonts for the character.
Can somebody help? This won't be fixed unless somebody with the correct
expertise helps us.
Comment 29•23 years ago
|
||
This is what I see when I look at comment # 6
in this bug.
Is it what should appear in the device manager window?
When I try to copy and paste this character from
the bug window to the pipnss.properties and recompile
pipnss.properties, the files doesn't load and
NSS default strings are used.
When I use \u0105, I get the little square.
I'll attach a picture of that.
Comment 30•23 years ago
|
||
Stephane, I think you are doing fine. \u105 is a Latin Extended. I think you
need to have a right font to display it. Let me dig out more info for you.
Reporter | ||
Comment 31•23 years ago
|
||
So You see right character, char in http://bugzilla.mozilla.org/attachment.cgi?id=58452&action=view
is ISO-8859-2 encoded "±" char.
Mayby this 32 byte strings should be in *.dtd file not *.properties?
Default encoding for *.dtd files is UTF8 (like in PKCS 11 specs) but for *.properties it is escape-unicode. Mayby thats why this strings are wrong displayed?
Comment 32•23 years ago
|
||
Well, there is only one unicode. It doesn't matter it come from UTF8 or escape
sequence. Internally browser converts them to unicode.
Comment 33•23 years ago
|
||
it seems to matter for this bug. I can add escape sequences, but not raw characters:
If I add the following to my pipnss.properties file:
PrivateTokenDescription=So±tware Securit\u0105 Device
then something goes wrong, and the values I get in the device manager are not
the values that come from the file, but from the hard-coded default values in
the NSS libraries, and that's true of all the strings from the device manager.
Reporter | ||
Comment 34•23 years ago
|
||
Another example, CRL validation, in "CRL settings" window I've got "." instead of some chars, but when I'm getting message box which says that new certificate is older (hmmm... this is only example) all chars are viewed properly.
Stephane Saux: I can make language packs which will be working for your build, please send me en-*.xpi and US.xpi. Or there is another method: Unpack all lang files and change in installed-chrome.txt and chrome.rdf files
jar:resource:/chrome/en-US.jar!/
jar:resource:/chrome/en-win.jar!/
jar:resource:/chrome/en-linux.jar!/
jar:resource:/chrome/en-mac.jar!/
jar:resource:/chrome/US.jar!/
to
resource:/chrome/
All strings will be taken from unpacked lang pack (after unpacking check directory structure - it should be the same as in *.xpi).
Reporter | ||
Comment 35•23 years ago
|
||
Comment 37•23 years ago
|
||
I think I have a fix for this bug.
The CRL issue is a separate one and a new bug should be filed for this.
Comment 38•23 years ago
|
||
When creating the software security device, take the ucs2 strings and convert
to UTF8.
Convert from utf8 to unicode for the token name when displaying in the
certManager.
Comment 39•23 years ago
|
||
kai please review.
Bug depends on 125728.
Assignee | ||
Comment 40•23 years ago
|
||
Comment on attachment 70214 [details] [diff] [review]
Convert to/from UTF8 when appropriate
The patch looks reasonable to me. As Bob said PKCS11 uses UTF8 internally, this
should work.
r=kaie
Attachment #70214 -
Flags: review+
Comment 41•23 years ago
|
||
Should we set the milestone to mozilla 1.0? since we have the fix at hand?
Comment 42•23 years ago
|
||
Setting milestone to 2.2
Unfortunately, the patch needs some additions to completely nail this.
I'll post a more comprehensive patch soon.
Target Milestone: Future → 2.2
Comment 43•23 years ago
|
||
Comment on attachment 70214 [details] [diff] [review]
Convert to/from UTF8 when appropriate
Needs work. need so change a few more places.
Attachment #70214 -
Flags: needs-work+
Comment 45•23 years ago
|
||
This handles all the places where pk11 strings need to be displayed:
certManager.
device manager.
set password.
get password.
change password.
Attachment #70214 -
Attachment is obsolete: true
Comment 46•23 years ago
|
||
Attachment #70997 -
Attachment is obsolete: true
Comment 47•23 years ago
|
||
Kai please review this patch.
Comment 48•23 years ago
|
||
*** Bug 113497 has been marked as a duplicate of this bug. ***
Comment 49•23 years ago
|
||
This patch fixes the issue I brought up in bug 113497. Thanks.
However, I would also like it you could add localization comments to the
properties file telling translators which values can only be 32/64 bytes long.
Otherwise, it is trial and error.
Assignee | ||
Comment 50•23 years ago
|
||
Stephane, the idea of adding comments to the locale files sounds like a good
idea to me, do you want to add them?
Comment 51•23 years ago
|
||
Attachment #71011 -
Attachment is obsolete: true
Comment 52•23 years ago
|
||
Kai, can you review.
Assignee | ||
Comment 53•23 years ago
|
||
Comment on attachment 76336 [details] [diff] [review]
Add localization comments to pipnss.properties.
>+ tokenName="_blank";
I don't understand why you do this in the exception case.
Is "_blank" the name of a special token?
If not, should we just set it to ""?
>- char *modNameCString = ToNewCString(modName);
>+ NS_ConvertUCS2toUTF8 modNameUTF8(modName);
> int modType;
>- SECMOD_DeleteModule(modNameCString, &modType);
>- SECMOD_AddNewModule(modNameCString, fullModuleName, 0, 0);
>+ SECMOD_DeleteModule((char *)modNameUTF8.get(), &modType);
>+ SECMOD_AddNewModule((char *)modNameUTF8.get(), fullModuleName, 0, 0);
I understand that NSS functions expect non-const pointers, and that you wanted
to avoid making a copy.
I would like to suggest, that instead of the hard cast, use:
NS_CONST_CAST(new-type, variable)
That way, you can search the sources for that kind of cast, when someone is
looking for potential problems.
>- mTokenLabel.AssignWithConversion((char *)tok_info.label,
>- sizeof(tok_info.label));
>+ nsCString tmpLabel((char *)tok_info.label, sizeof(tok_info.label));
>+ mTokenLabel = ToNewUnicode(NS_ConvertUTF8toUCS2(tmpLabel));
Your change is fine, but I'm not sure whether that extra ToNewUnicode call is
necessary.
Either this assignment transfers ownership of the value, then it is necessary.
But if the assignment operator of the target string class will create a copy,
then this call produces a leak.
We should find out.
In addition, to avoid another temporary copy of the string, you should not use
nsCString.
Please use nsDependentCString instead, which will create a read-only wrapper
object.
(both multiple times in your patch)
> NS_IMETHODIMP
> nsPKCS11Module::GetLibName(PRUnichar **aName)
> {
>+ if ( mModule->dllName ) {
> *aName = ToNewUnicode(NS_ConvertUTF8toUCS2(mModule->dllName));
>+ } else {
>+ *aName = NULL;
>+ }
In this case, the call to ToNewUnicode is correct.
Comment 54•23 years ago
|
||
Is this going to make it into Mozilla 1.0?
Comment 55•23 years ago
|
||
I need an additional fix.
Comment 56•23 years ago
|
||
I also added code to extract the largest possible uft8 string from strings
given to us by external libraries. Without this code NS_ConvertUTF8toUCS2 could
fail.
Attachment #76336 -
Attachment is obsolete: true
Comment 57•22 years ago
|
||
I'm posting a new patch that pulls the new functions out of nsNSSComponent.cpp
into new files.
We need this fix for RTM.
Whiteboard: [adt2 RTM]
Comment 58•22 years ago
|
||
Attachment #81964 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #83633 -
Flags: needs-work+
Assignee | ||
Comment 59•22 years ago
|
||
I'm taking the bug.
Let me summarize what the current patch does:
- moves token names to a parameter block, rather than abusing the window name of
a dialog
- add comments to inform translators of string size limits
- correct handling of token names, no longer handling them as C strings, but as
UTF-8
(Note, I yesterday already corrected this one on a larger scope in the
context of bug 144823)
- when converting from Unicode strings, preparing for giving them to NSS,
convert to UTF-8, rather than converting to plain ASCII strings.
I'm fine with the above, however the latest patch does more. Stephane, you are
trying to handle garbgage strings coming from the driver. I think this should
not be handled at the application level, but the NSS level should avoid
producing garbage strings.
In any case, that other problem is outside the scope of this bug, and I think we
should handle it separately. I have already filed bug 145228 and have proposed a
different solution. If you agree, please let's discuss it there.
I suggest that PSM should expect PKCS#11 strings returned from NSS be valid
UTF-8 strings.
I'll attach a new patch. Something else I want to change in the patch is using
NS_CONST_CAST instead of C-style casts.
Assignee: ssaux → kaie
Assignee | ||
Comment 60•22 years ago
|
||
Attachment #83633 -
Attachment is obsolete: true
Reporter | ||
Comment 61•22 years ago
|
||
Btw. Is this patch
(http://bugzilla.mozilla.org/showattachment.cgi?attach_id=84049) correct?
When I try to change password I'm getting FipsSlotDescription description
instead of PrivateTokenDescription (or InternalToken) in alert window. Attaching
screenshot. In other windows everything seems fine, dots are no longer displayed :)
Second is there any possibility to change 32bit length to 64bit? In some cases I
can't translate it (using only 32bit) without changing meaning of string :(
Reporter | ||
Comment 62•22 years ago
|
||
Reporter | ||
Comment 63•22 years ago
|
||
Seems that with new profile everything is fine ;)
Assignee | ||
Comment 64•22 years ago
|
||
*** Bug 130988 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 65•22 years ago
|
||
After some more discussion with Stephane:
- the previous alone makes siuation a bit worse with some pkcs#11 drivers,
because we now do UTF-8 conversion, we would display empty strings instead of
strings with some garbage in the middle. Therefore Stephane suggested that this
should be blocked by bug 145228
- however, I don't want to wait on the NSS bug to be fixed before 90956 lands
- while the patch in bug 145228 makes sense to add, I meanwhile learned more
about the actual problem. I believe the strings returned from pkcs#11 are
always either zero terminated or blank padded, i.e. at least one of both. (I
believe if a driver does neither of both, that is a really really buggy driver,
and while working around those drivers might make sense (as suggested with bug
145228), I think that should not block this bug 90956).
- however, I do agree that we should care for the likelyhood of a zero
terminated buffer received from the driver at the application level.
Therefore I'm attaching a new patch, that still uses a substring of the whole
buffer, if there is no zero termination, but if there is a zero termination, it
uses the shorter substring, and therefore the UTF-8 conversion will correctly
work up to that position.
Attachment #84049 -
Attachment is obsolete: true
Comment 66•22 years ago
|
||
Comment on attachment 85541 [details] [diff] [review]
Suggested Patch v7
r=ssaux
This patch is very important for localization.
Attachment #85541 -
Flags: review+
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 67•22 years ago
|
||
I asked Alec to sr on AIM, and he did not like the wrapper class I introduced.
He prefers avoiding it and suggested a way to get rid of it.
Basically, the only reason for nsBuffer2DependentCSubstring's existence was to
find the valid length of the buffer. It could be either the maximum, if the
buffer is fully filled with non-null bytes, or a shorter length, if there is a
null found in the string.
Alec recommended to use PL_strnlen instead, which does exactly that.
I'm attaching a new patch, that has no longer the wrapper class, and has the
expressions rewritten to use PL_strnlen.
Attachment #85541 -
Attachment is obsolete: true
Comment 68•22 years ago
|
||
Comment on attachment 88525 [details] [diff] [review]
Patch v8
sr=alecf
Attachment #88525 -
Flags: superreview+
Assignee | ||
Updated•22 years ago
|
Attachment #88525 -
Flags: review+
Assignee | ||
Comment 69•22 years ago
|
||
Comment on attachment 88525 [details] [diff] [review]
Patch v8
carrying over r=ssaux
Updated•22 years ago
|
Whiteboard: [adt2 RTM] → [adt2 RTM] [eta 6/24]
Assignee | ||
Comment 70•22 years ago
|
||
Checked in to trunk.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•22 years ago
|
Keywords: adt1.0.1,
mozilla1.0.1
Comment 71•22 years ago
|
||
junruh - can you pls verify this fix on the trunk, the resolve as verified? thanks!
Blocks: 143047
Comment 73•22 years ago
|
||
Andreas B.
You mentioned you could test with the Jap. language pack. When could this be done?
Thx.
stephane.
Reporter | ||
Comment 74•22 years ago
|
||
Working fine on Polish build, attaching screenshoot :)
Thnx for fix :)
Reporter | ||
Comment 75•22 years ago
|
||
Comment 76•22 years ago
|
||
Stephane, we would need a Japanese language pack which is based on the latest
trunk build. rchen, yxia, is this something you could help us with?
Comment 77•22 years ago
|
||
verifying for l10n based on Comment #75 From Marek Wawoczny, and the attachment.
looks good!
adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch, pending
drivers approval. pls check this in asap, then add the "fixed1.0.1" keyword.
Comment 78•22 years ago
|
||
Andreas,
apparently, japanese is only available for branch builds. The test with polish
that Marek performed (see comment #74 and #75) shows that using a conversion
from UTF8 is the right thing to do. The Polish test already shows that
multibyte utf8 encoded strings do work properly. After that, the japanese test
could only reveal problems in Moz utf8 conversion routines, which have been
extensively tested outside of PSM.
I would certainly welcome as early a test of japanese on the branch as is possible.
Updated•22 years ago
|
Attachment #88525 -
Flags: approval+
Comment 79•22 years ago
|
||
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Keywords: mozilla1.0.1 → mozilla1.0.1+
Comment 81•22 years ago
|
||
Verified that the code was checked into the branch.
Keywords: fixed1.0.1 → verified1.0.1
Comment 82•22 years ago
|
||
verified on the branch with 2002-06-24-1.0.0 build +japanse langpack. All PSM
dlgs look correct to me. Cc'ing iqa
Comment 83•22 years ago
|
||
*** Bug 155565 has been marked as a duplicate of this bug. ***
Comment 84•18 years ago
|
||
Please use the standard notation for localization warnings:
LOCALIZATION NOTE (<entityname>): <explanation>
This will help a lot with automated tools, thanks!
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•