RocketChat / Rocket.Chat

The communications platform that puts data protection first.
https://rocket.chat/
Other
40.27k stars 10.44k forks source link

invalid percent-encoding error when routing url includes "%c0" #33027

Open gizembg opened 2 months ago

gizembg commented 2 months ago

Description:

Hi, when I try to go an url like "https://cloud.rocket.chat/%c0" app shows on white screen. It throws an error on console:

_URIError: Pathname "/%C0" could not be decoded. This is likely caused by an invalid percent-encoding._

Steps to reproduce:

  1. Go to 'https://cloud.rocket.chat'
  2. login to your account
  3. go to https://cloud.rocket.chat/%c0

Expected behavior:

See 404 error instead of white screen.

Actual behavior:

rocketchat

Client Setup Information

Additional context

Can you please explain where this log fired from? I am trying to redirect to another page when user enters such url includes "%c0" instead of white screen.

reetp commented 2 months ago

Thanks for reporting.

I'll report it to the internal teams.

casalsgh commented 2 months ago

gizembg can you share a bit more on what you are trying to do? tend to see this as more of a really edge case and don't quite see need and use, so please enlighten me.

gizembg commented 2 months ago

Hi @casalsgh

I've encountered an issue where my app crashes when I input %c0 into the URL. This triggers the match function in connect-route.js under:

rocket.chat/apps/meteor/.meteor/local/isopacks/rocketchat_restivus/npm/node_modules/connect-route/lib/connect-route.js

Details:

Here's the match function:

    Router.prototype.match = function (method, url) {  
        var  
            parts = decodeURI(url).split('?', 1)[0].replace(separator, '').split('/'),  
            result = { handler: undefined, route: undefined, params: {} },  
            current, i, length;

            if (this.routes[method]) {  
            if (!parts[0].length) {  
                result.handler = this.routes[method].handler;  
                result.route = this.routes[method].route;  
            } else {  
                current = this.routes[method];  
                for (i = 0, length = parts.length; i < length; i++) {  
                    if (current.childs[parts[i]]) {  
                        current = current.childs[parts[i]];  
                        result.handler = current.handler;  
                        result.route = current.route;  
                    } else if (current.childs['*'] && current.childs['*'][length]) {  
                        current = current.childs['*'][length];  
                        result.handler = current.handler;  
                        result.route = current.route;  
                        result.params[current.name] = parts[i];  
                    } else {  
                        result.handler = undefined;  
                        result.route = undefined;  
                        result.params = {};  
                        break;  
                    }  
                }  
            }  
        }  
        return result;  
    };  

Problem:

The issue seems to occur when the match function tries to encode certain characters. Do you have any solutions or workarounds to prevent this crash? If not, could you share how you handle this situation in your code? Specifically, I noticed that an error is thrown—could you point me to where this error-throwing logic is implemented so I can apply a similar fix?

Looking forward to your insights!

reetp commented 2 months ago

Hi @casalsgh

You don't need @ people thanks.

The issue seems to occur when the match function tries to encode certain characters. Do you have any solutions or workarounds to prevent this crash? If not, could you share how you handle this situation in your code? Specifically, I noticed that an error is thrown—could you point me to where this error-throwing logic is implemented so I can apply a similar fix?

This would be a backend issue. I'm not sure they are going to share any of their internal code.

The short solution is don't artificially insert odd codes for no reason.

I've encountered an issue where my app

I'm also not sure what your app is doing.

I don't think cloud is meant to be accessed via home built apps?

Tell us a lot more about what you are doing.

casalsgh commented 2 months ago

Go a reply from one of the internal engineers analyzing the decoding from: https://www.urldecoder.org/ ; didn't quite work on UTF-8 or ASCII. Overall function there will not decodify it.

If you want to propose a change or PR for it, please do and I'll see that it gets reviewed. but at the time from internal teams this quite the edge case and will be a "won't fix".

julio-cfa commented 1 month ago

It seems to be an edge case. Do you have any situations where you'd need to use URL-encoded chars such as %C0? The error seems to happen because the function being used doesn't support charsets such as Windows-1252, but that shouldn't be a problem for most cases.