427089 - FTP shouldn't send URLs in UTF-8

Status: RESOLVED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Nickolay Zagorodnev]

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

[Matthias Versen [:Matti]]

An example URL would be nice

Comment 2

[Nickolay Zagorodnev]

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

[Matthias Versen [:Matti]]

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/

Comment 4

[Michal Novotny [:michal]]

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.

Comment 5

[Michal Novotny [:michal]]

(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.

Comment 6

[Michal Novotny [:michal]]

Attachment: convert path from UTF8 to originCharset

(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.

Comment 8

[shattered]

see also bug 297395 with similar patch

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

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

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

I prefer this patch over bug 297395's

Comment 11

[Michal Novotny [:michal]]

Attachment: new patch

(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?

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

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

Comment 13

[Michal Novotny [:michal]]

Attachment: patch including fix for nsStandardURL

Comment 14

[Dão Gottwald [:dao]]

http://hg.mozilla.org/index.cgi/mozilla-central/rev/0c0e0cdd761a

Comment 16

[Владимир]

When there will be a updating solving this problem?

Comment 17

[Matthias Versen [:Matti]]

Firefox 3.1 (not 3.01 and not 3.0.4)