erlyaws / yaws

Yaws webserver
https://erlyaws.github.io
BSD 3-Clause "New" or "Revised" License
1.28k stars 267 forks source link

yaws_api:url_encode/1 Exception: function_clause #329

Closed leoliu closed 5 years ago

leoliu commented 6 years ago

Could yaws_api:url_encode/1 be made to support binary? I often get a crash like this:

ERROR erlang code threw an uncaught exception:
 File: appmod:0
Class: error
Exception: function_clause
Req: {http_request,'GET',{abs_path,"/"},{1,1}}
Stack: [{yaws_api,url_encode,
leoliu commented 6 years ago

There are other issues with that function. For example it converts URL to binary according to file:native_name_encoding() which is only correct when serving files from the system. But request can also be handled by an appmod i.e. it has nothing to do with file path. It all accidentally works out because the emulator defaults to utf8.

vinoski commented 5 years ago

The call to file:native_name_encoding/0 is an attempt to guess the native encoding of the system. Yes, it's flawed, since the +pc startup option to the erl emulator can change things. Perhaps using io_lib:printable_list/1 to decide about the URL argument would be better, but I'd never claim to be an expert in these matters.

As for supporting binary, sure, I'll try to add that over the next few days.

vinoski commented 5 years ago

Please give this branch a try when you get a chance, feedback welcomed.

leoliu commented 5 years ago

Seems to be working well. Thanks.

leoliu commented 5 years ago

@vinoski I just discovered a bug from this commit. 404 is returned for any request path that has unicode chars in it. For example place a file 你好.txt in a directory that is served by yaws and try to view it.

The requested URL /%E4%BD%A0%E5%A5%BD.txt was not found on this server.
vinoski commented 5 years ago

Could you open a new issue for that? It's easier to track that way. Thanks.