security(polymarket-browse): use proper URL encoding for --search parameter #34

shoko · 2026-03-26T20:12:19+01:00

shoko commented

2026-03-26 20:12:19 +01:00

Summary

Replace manual space replacement with proper URL encoding for --search parameter.

Changes

Import quote from urllib.parse
Replace q.replace(" ", "%20") with quote(q, safe="")

Before

Only spaces were encoded. Special chars like &, =, %, # could cause URL injection.

After

All special characters are properly encoded:

& -> %26
= -> %3D
% -> %25
+ -> %2B
# -> %23

Testing

70/70 tests passing

## Summary Replace manual space replacement with proper URL encoding for --search parameter. ## Changes - Import `quote` from `urllib.parse` - Replace `q.replace(" ", "%20")` with `quote(q, safe="")` ## Before Only spaces were encoded. Special chars like `&`, `=`, `%`, `#` could cause URL injection. ## After All special characters are properly encoded: - `&` -> `%26` - `=` -> `%3D` - `%` -> `%25` - `+` -> `%2B` - `#` -> `%23` ## Testing 70/70 tests passing

shoko added 1 commit 2026-03-26 20:12:19 +01:00

security(polymarket-browse): use proper URL encoding for --search parameter bb7eebf502

- Import quote from urllib.parse
- Replace q.replace(' ', '%20') with quote(q, safe='')
- Properly encodes: &, =, %, +, #, ?, and other special chars
- Prevents URL injection attacks

shoko reviewed 2026-03-27 01:51:03 +01:00

shoko left a comment

Review requested

shoko added the need review label 2026-03-27 01:52:45 +01:00

han commented

2026-03-27 02:09:15 +01:00

what happen with symbols like (, ), +, -, ", etc can you create unit test cases to cover these symbols? I think testing only the q.replace functions should be sufficient. what do you think?

han added need adjustment and removed need review labels 2026-03-27 02:09:23 +01:00

shoko added 1 commit 2026-03-27 02:14:55 +01:00

test(polymarket-browse): add URL encoding unit tests 3016d1287c

Add TestUrlEncoding class testing quote() encodes:
- Space -> %20
- & -> %26
- = -> %3D
- % -> %25
- + -> %2B
- ( -> %28
- ) -> %29
- # -> %23

shoko commented

2026-03-27 02:15:13 +01:00

Good point! I added unit tests for URL encoding of special characters:

Test Coverage Added

Symbol	Encoded As
Space	%20
&	%26
=	%3D
%	%25
+	%2B
(	%28
)	%29
#	%23

Test Code

def test_quote_encodes_special_chars(self):
    from urllib.parse import quote
    test_cases = [
        ("Team A", "Team%20A"),
        ("Team A & Team B", "Team%20A%20%26%20Team%20B"),
        ("a=b", "a%3Db"),
        ("100%", "100%25"),
        ("C++", "C%2B%2B"),
        ("Team (A)", "Team%20%28A%29"),
        ("Team#1", "Team%231"),
    ]
    for input_str, expected in test_cases:
        self.assertEqual(quote(input_str, safe=""), expected)

Tests pass: 71/71 passing

The issue was that the old code only did q.replace(" ", "%20") which only encoded spaces. The fix uses quote(q, safe="") which properly encodes all special characters.

Good point! I added unit tests for URL encoding of special characters: ## Test Coverage Added | Symbol | Encoded As | |--------|------------| | Space | %20 | | & | %26 | | = | %3D | | % | %25 | | + | %2B | | ( | %28 | | ) | %29 | | # | %23 | ## Test Code ```python def test_quote_encodes_special_chars(self): from urllib.parse import quote test_cases = [ ("Team A", "Team%20A"), ("Team A & Team B", "Team%20A%20%26%20Team%20B"), ("a=b", "a%3Db"), ("100%", "100%25"), ("C++", "C%2B%2B"), ("Team (A)", "Team%20%28A%29"), ("Team#1", "Team%231"), ] for input_str, expected in test_cases: self.assertEqual(quote(input_str, safe=""), expected) ``` Tests pass: **71/71 passing** The issue was that the old code only did `q.replace(" ", "%20")` which only encoded spaces. The fix uses `quote(q, safe="")` which properly encodes all special characters.

shoko added need review and removed need adjustment labels 2026-03-27 02:16:08 +01:00

han approved these changes 2026-03-27 02:24:42 +01:00

han left a comment

lgtm

han added approved and removed need review labels 2026-03-27 02:24:53 +01:00

shoko closed this pull request

2026-03-27 04:09:43 +01:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: shoko/jujutsu-skills#34