Closed
Bug 427089
Opened 16 years ago
Closed 16 years ago
FTP shouldn't send URLs in UTF-8
Categories
(Core Graveyard :: Networking: FTP, defect)
Core Graveyard
Networking: FTP
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9.1a2
People
(Reporter: brutforce, Assigned: michal)
References
Details
Attachments
(1 file, 2 obsolete files)
7.11 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.9b5) Gecko/2008032620 Firefox/3.0b5 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.9b5) Gecko/2008032620 Firefox/3.0b5 "550 Failed to change directory" message box appear if I chose any folder that contain Russian characters. Reproducible: Always Steps to Reproduce: 1. 2. 3.
Comment 1•16 years ago
|
||
An example URL would be nice
Component: File Handling → Networking: FTP
Product: Firefox → Core
QA Contact: file.handling → networking.ftp
Reporter | ||
Comment 2•16 years ago
|
||
for example ftp://ftp.asu.ru/incoming/ Russain name folders are not available in FireFox :( This problem take place in other places too.
Comment 3•16 years ago
|
||
This seems to be a char escape problem. example: ñêèí is escaped to ftp://ftp.asu.ru/incoming/%C3%B1%C3%AA%C3%A8%C3%AD while IE escapes this to ftp://ftp.asu.ru/incoming/%F1%EA%E8%ED/
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 4•16 years ago
|
||
The problem is somewhere else. Directory with name "ñêèí" is actually "скин". First name is in ISO-8859-1 which is incorrectly auto-detected. The correct one is in cp1251 which can be set manually. String %C3%B1%C3%AA%C3%A8%C3%AD is escaped UTF-8 that was converter from ISO-8859-1 and is obviously wrong. Correct UTF-8 escaped string is %D1%81%D0%BA%D0%B8%D0%BD. But even URL "ftp://ftp.asu.ru/incoming/%D1%81%D0%BA%D0%B8%D0%BD/" doesn't work because that FTP server doesn't understand UTF-8. The only way to follow the link that I found is to disable UTF-8 encoding of URL (network.standard-url.encode-utf8 in about:config) and to set charset to cp1251 manually. In this case the link is "ftp://ftp.asu.ru/incoming/%F1%EA%E8%ED/". Correct solution would be probably to convert FTP path always to original charset in nsFtpState::Init() after it is unescaped. The problem is IMHO that there is no information about current charset of the path. String obtained from nsIURI::GetPath() can be encoded either in mOriginCharset (if network.standard-url.encode-utf8 == false) and then no conversion is needed or in UTF-8 and then conversion is needed. I couldn't find a way to find out if conversion should be performed.
Assignee | ||
Comment 5•16 years ago
|
||
(In reply to comment #4) > FTP server doesn't understand UTF-8. The only way to follow the link that I > found is to disable UTF-8 encoding of URL (network.standard-url.encode-utf8 in > about:config) and to set charset to cp1251 manually. In this case the link is > "ftp://ftp.asu.ru/incoming/%F1%EA%E8%ED/". This is not correct. Selecting cp1251 manually isn't needed and disabling "network.standard-url.encode-utf8" is enough to get it working.
Assignee | ||
Comment 6•16 years ago
|
||
(In reply to comment #4) > in UTF-8 and then conversion is needed. I couldn't find a way to find out if > conversion should be performed. I think that it is safe to try to do the conversion always when IsUTF8(mPath) is true. The patch implements it. It will work in most cases. I found following problematic scenario: 1) Assume that there is a file with URL ftp://localhost/скин/file and it is encoded in cp1521. 2) User gets listing of ftp://localhost/ and charset is badly detected as ISO-8859-1 ("ñêèí" is displayed instead of "скин"). 3) User switches encoding to cp1251 and "скин" is now displayed correctly. 4) User clicks on "скин" and gets listing of ftp://localhost/скин/ and charset is again set to ISO-8859-1. 5) Because "file" isn't in cyrillic, user don't switch to cp1251 and clicks on "file". 6) Now there is a problem because path can't be converted to ISO-8859-1. UTF8 path will be used instead. FTP server can't find such file so FF will think that it is a directory and tries to get listing but this fails too. The file can't be downloaded due to bug #406787 even if user will switch now to cp1251.
see also bug 297395 with similar patch
Comment 9•16 years ago
|
||
Comment on attachment 328496 [details] [diff] [review] convert path from UTF8 to originCharset + originCharset != NS_LITERAL_CSTRING("UTF-8")) !originCharset.EqualsLiteral("UTF-8") + nsAutoString ucsPath = NS_ConvertUTF8toUTF16(mPath); NS_ConvertUTF8toUTF16 ucsPath(mPath); I'd also add an NS_ASSERTION that verifies IsUTF8(mPath) + result = p; can you make this result.Assign(p)? that makes it clearer that this is a string class without having to scroll up + rv = encoder->Finish(p, &len); So, this code is a bit confusing and I think not quite correct (you're passing in a strange length). Why not just pass in buf and set len to sizeof(buf) - 1? + encoder->Reset(); don't see much point in this considering that the encoder is about to be destroyed anyway
Attachment #328496 -
Flags: review?(cbiesinger) → review+
Comment 10•16 years ago
|
||
I prefer this patch over bug 297395's
Updated•16 years ago
|
OS: Windows XP → All
Hardware: PC → All
Summary: "Failed to change directory" in FTP-client → FTP shouldn't send URLs in UTF-8
Assignee | ||
Comment 11•16 years ago
|
||
(In reply to comment #9) > + rv = encoder->Finish(p, &len); > > So, this code is a bit confusing and I think not quite correct (you're passing > in a strange length). Why not just pass in buf and set len to sizeof(buf) - 1? You are right. I took this code from http://mxr.mozilla.org/mozilla/source/netwerk/base/src/nsStandardURL.cpp#91 so it needs to be fixed too. Is it possible to fix that code within this bug too?
Attachment #328496 -
Attachment is obsolete: true
Attachment #331478 -
Flags: superreview?(darin.moz)
Comment 12•16 years ago
|
||
Comment on attachment 331478 [details] [diff] [review] new patch + nsCOMPtr<nsICharsetConverterManager> charsetMgr ( should remove the space before the ( darin isn't really doing reviews anymore, so sr=biesi feel free to fix standardurl in this bug too
Attachment #331478 -
Flags: superreview?(darin.moz) → superreview+
Assignee | ||
Comment 13•16 years ago
|
||
Attachment #331478 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Comment 14•16 years ago
|
||
http://hg.mozilla.org/index.cgi/mozilla-central/rev/0c0e0cdd761a
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a2
Comment 16•16 years ago
|
||
When there will be a updating solving this problem?
Comment 17•16 years ago
|
||
Firefox 3.1 (not 3.01 and not 3.0.4)
Updated•2 months ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•