-
-
Notifications
You must be signed in to change notification settings - Fork 627
fix: add urls with space #3675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
fix: add urls with space #3675
Conversation
Signed-off-by: codomposer <akileus0902@gmail.com>
|
This may not be the best approach for a couple of reasons. URLs cannot contain spaces but they can contain commas (https://datatracker.ietf.org/doc/html/rfc3986#section-2). Other fields in the webUI use both spaces and newlines as field delimiters, this change would have different behaviour in different text input fields. There seems to be a simpler fix which to allow people to enter URLs containing %20 for a "space", without interfering with other URLs or altering the accepted delimiters:
|
Signed-off-by: codomposer <akileus0902@gmail.com>
Signed-off-by: codomposer <akileus0902@gmail.com>
0f99bc6 to
e74d0ff
Compare
|
@rrobgill |
New approach tests ok for me. |
then can you merge it? |
|
@codomposer please don't ping people for merge - PRs are prioritised on a "when we can get to it" basis, which is just the unfortunate nature of having to balance "real jobs" and this one. (besides, rrobgill is a contributor but does not have merge rights) |



Thank you for your contribution to the Pi-hole Community!
Please read the comments below to help us consider your Pull Request.
We are all volunteers and completing the process outlined will help us review your commits quicker.
Please make sure you
What does this PR aim to accomplish?:
Closes #3364
Fix: URLs with spaces can now be added to blocklist/allowlist
Problem
%20encoding resulted in double-encoding to%2520/[\s,]+/treated spaces as delimitersHow does this PR accomplish the above?:
Solution
/[\s,]+/to/[,\n]+/ingroups-lists.jsLink documentation PRs if any are needed to support this PR:
By submitting this pull request, I confirm the following:
git rebase)Contribution by Gittensor, learn more at https://gittensor.io/