8000 Adds support for the Content-Encoding header by aminroosta · Pull Request #256 · nicolasff/webdis · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Adds support for the Content-Encoding header #256

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Adds support for the Content-Encoding header #256

wants to merge 6 commits into from

Conversation

aminroosta
Copy link

Fixes #255

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 16, 2025
View reviewed changes
@nicolasff
Copy link
Owner

Hi Amin, thanks for the PR! I've only given it a brief look so far but will spend some time on it today.

In the meantime I did trigger the tests which run on a range of systems, they're all marked as failed simply because the same test failed in all environments; so it looks all red but it is really just about one test :-)

From the output:

FAIL: test_encoding (__main__.TestJSON.test_encoding)
success type (+OK)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/__w/webdis/webdis/./tests/basic.py", line 87, in test_encoding
    self.assertTrue(f.getheader('ETag') == '"fcb2917ae[14](https://github.com/nicolasff/webdis/actions/runs/13878036214/job/38846608045?pr=256#step:9:15)a62e911fbfb2b8ea66800"')
AssertionError: False is not true

To run this validation yourself, start Webdis with websockets enabled (set "websockets": true in webdis.json):

-  "websockets": false,
+  "websockets": true,

and in the tests/ directory run ./basic.py and ./ws-tests.py, that's usually how I run an initial check over Webdis.

You'll need to install websocket-client>=1.1.0 for Python, I suggest you create a venv to leave your global Python install untouched. With Redis and Webdis already running:

cd tests
python -m venv .venv
source .venv/bin/activate
pip install -r requirements.txt

./basic.py
./ws-tests.py

@aminroosta
Copy link
Author

Hello @nicolasff, I appreciate you taking the time to review the PR.
I've made a tiny change in ec025f8 to support both key.json.gzip and key.gzip.
I hope the pointer arithmetic is accurate 🤞.

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 17, 2025
View reviewed changes
src/cmd.c
suffix = ext + 1;
suffix_len = uri + uri_len - suffix;

for(ext = ext - 1; ext != uri && *ext != '/'; --ext) {

Check notice

Code scanning / CodeQL

For loop variable changed in body Note

Loop counters should not be modified in the body of the
loop
Loading
.
src/cmd.c
suffix = ext + 1;
suffix_len = uri + uri_len - suffix;

for(ext = ext - 1; ext != uri && *ext != '/'; --ext) {

Check notice

Code scanning / CodeQL

For loop variable changed in body Note

Loop counters should not be modified in the body of the
loop
Loading
.
src/cmd.c

for(ext = ext - 1; ext != uri && *ext != '/'; --ext) {
if(*ext == '.') {
ext++;

Check notice

Code scanning / CodeQL

For loop variable changed in body Note

Loop counters should not be modified in the body of the
loop
Loading
.
Loop counters should not be modified in the body of the
loop
Loading
.
@aminroosta
Copy link
Author

Will fix the unit tests later today. The python gzip implementation includes a timestamp.

@aminroosta
Copy link
Author

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Content-Encoding: gzip
2 participants
@aminroosta @nicolasff
0