Skip to content

Conversation

@Anuskuss
Copy link

Are you able to read this: https://pi.hole/queries?client_ip=fe80%3A%3A3aa3%3Aa33a%3A3a3a%3Aa3a3?
Well neither am I.

(Feel free to close and "steal" the code. It's a single line anyway.)

Signed-off-by: Anuskuss <anuskuss@googlemail.com>
@Anuskuss Anuskuss requested a review from a team as a code owner December 24, 2025 00:01
@DL6ER
Copy link
Member

DL6ER commented Dec 25, 2025

I see in which direction you are heading but I'm not agreeing on the solution. The encodeURI and encodeURIComponent - while both functions are used for encoding - serve different purposes and encode different sets of characters. That's why you don't see the encoding of : -> %3A with the former.

encodeURI does not acknowledge query strings, it is only used for files, i.e., if we are constructing an URI, we would need to use something like

const url = encodeURI("https://pi.hole/queries?client_ip=") + encodeURIComponent(the_ip_address);

We leave off the former encodeURI because we know at this point that the result of the former doesn't change under encodeURI but this doesn't mean we should be using this function also for the query parameter. It is quite obvious why the latter doesn't transform : -> %3A because that'd destroy a possible port specification after the domain as well as the protocol specifier (http://).

This function should replace further : down the line but it is really implemented as a simple regex replacer so it isn't aware of which part is where.

@DL6ER
Copy link
Member

DL6ER commented Dec 25, 2025

Quoting from MDN:

Compared to encodeURI(), encodeURIComponent() escapes a larger set of characters. Use encodeURIComponent() on user-entered fields from forms sent to the server — this will encode & symbols that may inadvertently be generated during data entry for character references or other characters that require encoding/decoding. For example, if a user writes Jack & Jill, without encodeURIComponent(), the ampersand could be interpreted on the server as the start of a new field and jeopardize the integrity of the data.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent

@Anuskuss
Copy link
Author

I don't think that IP addresses need to be encoded at all because they can only contain [0-9A-F.:] but then again I don't know if a malicious actor is able to somehow create an attack based on that. I only changed it to encodeURI() because it was a quick fix for a problem I had while still being mostly correct and also theoretically handling the percent sign in e.g. fe80::1%eth0, but I agree that encodeURIComponent() is the proper way to do this.
So if you don't like this change I'd suggest you instead change it to:

encodeURIComponent(client.ip).replace('%3A',':')
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

2 participants