expressjs / express

Fast, unopinionated, minimalist web framework for node.
https://expressjs.com
MIT License
65.8k stars 16.45k forks source link

How to process url with '%' ? #2947

Closed syter closed 8 years ago

syter commented 8 years ago

I have used express 3.3.1 for a nodejs server which provides restful API.

In this server, I provide functions that are upload photo, get photo data from url. When upload photo, server will save photo data to mongodb, and response url that cloud visit photo data directly. for example: request: photo:123.jpg response: url:http://192.168.1.9:3000/v2/resources/56ea1d4df9df0d6b090b12b1/123.jpg

But there is an issue I can't fix it. When upload photo name is ']G207E5A%7DC(%7B%K$(2~.jpg' such this thing, upload photo is successful, but visit url http://192.168.1.9:3000/v2/resources/56ea1d4df9df0d6b090b12b1/]_G207E5A_%7DC(%7B%K$(2~.jpg is failed.

the server logs: [2016-03-17 12:55:00.028] [DEBUG] mrm - { [Error: Bad Request] status: 400 } Error: Bad Request at SendStream.error (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/node_modules/express/node_modules/send/lib/send.js:145:16) at SendStream.pipe (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/node_modules/express/node_modules/send/lib/send.js:298:31) at Object.handle (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/node_modules/express/node_modules/connect/lib/middleware/static.js:84:8) at next (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/node_modules/locomotive/node_modules/express/node_modules/connect/lib/proto.js:190:15) at Object.logger (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/node_modules/express/node_modules/connect/lib/middleware/logger.js:156:5) at next (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/node_modules/locomotive/node_modules/express/node_modules/connect/lib/proto.js:190:15) at Object.handle (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/lib/WAF/config/environments/all.js:44:3) at next (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/node_modules/locomotive/node_modules/express/node_modules/connect/lib/proto.js:190:15) at Object.expressInit as handle at next (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/node_modules/locomotive/node_modules/express/node_modules/connect/lib/proto.js:190:15) at Object.query as handle at next (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/node_modules/locomotive/node_modules/express/node_modules/connect/lib/proto.js:190:15) at Function.app.handle (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/node_modules/locomotive/node_modules/express/node_modules/connect/lib/proto.js:198:3) at Server.app (/Users/Syter/Documents/workcode/myFolk/arrownock-mrm/node_modules/locomotive/node_modules/express/node_modules/connect/lib/connect.js:65:37) at emitTwo (events.js:87:13) at Server.emit (events.js:172:7) at HTTPParser.parserOnIncoming as onIncoming at HTTPParser.parserOnHeadersComplete (_http_common.js:88:23) 192.168.1.9 - - [Thu, 17 Mar 2016 04:55:00 GMT] "GET /favicon.ico HTTP/1.1" 400 129 "http://192.168.1.9:3000/v2/resources/56ea1d4df9df0d6b090b12b1/]_G207E5A_%7DC(%7B%K$(2~.jpg" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36"

I found this issue only appears, when url with '%' in it. So how can I fix it?

blakeembrey commented 8 years ago

@syter A percent sign is used for encoding URLs. You can read more information about this: https://en.wikipedia.org/wiki/Percent-encoding. To encode strings correctly for use in a URL, you want to encode the string yourself. For example, encodeURIComponent('/%') becomes %2F%25. Notice the percent signs indicate something that needs to be "decoded". The reason this fails for you is because decodeURIComponent('%K$') does not mean anything - it's a malformed URL.

syter commented 8 years ago

@blakeembrey Thanks for your answers. I know '%' is used for encoding URLs. But you know this server's destiny is provided lots of RESTFUL APIs, so upload photo and get photo functions will be called by any developers. Some developers use these strange name photos, that I can't prevent. What can I do is provided a parameter 'filename' to update photo name in my db.

For example: curl -F "photo=@]G207E5A%7DC(%7B%K$(2~.jpg" -F "filename=123.jpg" "http://xxxxxxxxx/photos/create.json" and the response is: url:http://192.168.1.9:3000/v2/resources/56ea1d4df9df0d6b090b12b1/123.jpg this url is visit successfully.

But there are some developers still upload these such strange name photos. So my willing is how could this server response photo data even photo name with '%'?

hacksparrow commented 8 years ago

How about converting all non-alphanumeric characters to dashes? So GA%123.jpg becomes GA-123.jpg etc. Or maybe just strip them off?

The approach of preserving any kind of file name and and ability to print them can become a source of XSS and other web vulnerabilities. Do not support non-alphanumeric characters in file names. On Thu, 17 Mar 2016 at 11:22 AM, Syter notifications@github.com wrote:

@blakeembrey https://github.com/blakeembrey Thanks for your answers. I know '%' is used for encoding URLs. But you know this server's destiny is provided lots of RESTFUL APIs, so upload photo and get photo functions will be called by any developers. Some developers use these strange name photos, that I can't prevent. What can I do is provided a parameter 'filename' to update photo name in my db.

For example: curl -F "photo=@]G207E5A%7DC(%7B%K$(2~.jpg" -F "filename=123.jpg" " http://xxxxxxxxx/photos/create.json" and the response is: url:http://192.168.1.9:3000/v2/resources/56ea1d4df9df0d6b090b12b1/123.jpg this url is visit successfully.

But there are some developers still upload these such strange name photos. So my willing is how could this server response photo data even photo name with '%'?

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/expressjs/express/issues/2947#issuecomment-197718114

syter commented 8 years ago

@hacksparrow Do you mean when developers upload photos, server check the photo name, if there are non-alphanumeric characters in it, replace or remove it first, then save it to DB, so it will no longer appears '%' in url anymore? Sounds it's a good idea, I think if anyone upload a photo with meanless name, they don't care about the filename really. I will try it. I will let you know if it's OK or not.

Thank you.

hacksparrow commented 8 years ago

Yes, that's what I meant. However, instead of manually doing a conditional if check, you can use regex to process all the file names.

var fileName = originalName.replace(/\W+/g, '-'); // replace with dashes
var fileName = originalName.replace(/\W+/g, ''); // remove invalid characters completely

Note: The second approach will end up with an empty string, if the original file name was composed of all invalid characters. Put something in place to take care of such probabilities.

syter commented 8 years ago

Everything is fine. Thank you.