Skip to content

Conversation

lucaswitch
Copy link

Add support to request to parse gzip encoded posts.

Q A
Is bugfix?
New feature? ✔️
Breaks BC? ❌

Add support to request to parse gzip encoded posts.
Copy link

codecov bot commented Aug 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.47%. Comparing base (36d67cd) to head (55151ea).

Additional details and impacted files
@@            Coverage Diff            @@
##             master   #20477   +/-   ##
=========================================
  Coverage     64.47%   64.47%           
- Complexity    11574    11579    +5     
=========================================
  Files           433      433           
  Lines         37598    37606    +8     
=========================================
+ Hits          24240    24248    +8     
  Misses        13358    13358           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@terabytesoftw
Copy link
Member

Thanks for the PR, to avoid creating BC, i left the gzip option set to false, and it would be great if you added a unit test to verify its functionality.

Copy link
Member

@samdark samdark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Please add a line for CHANGELOG. Also, is it possible to add some unit tests for it?

@lucaswitch
Copy link
Author

@samdark @terabytesoftw Tests included.

Still there is a scenario left.
Should be multiple combinated compression algorithms be supported in yii2?
The case of: Content-Encoding: gzip, deflate.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Encoding

@samdark
Copy link
Member

samdark commented Aug 12, 2025

@lucaswitch how did you encounter the issue this pull request is solving btw.? Which server was it? Majority of servers are decoding gzip transparently and automatically.

@lucaswitch
Copy link
Author

lucaswitch commented Aug 12, 2025

@lucaswitch how did you encounter the issue this pull request is solving btw.? Which server was it? Majority of servers are decoding gzip transparently and automatically.

They do handle it but on the responses. When incoming requests are done with Content-Encoding: gzip it leaves for the app to handle the encoding processing.

@lucaswitch
Copy link
Author

@lucaswitch how did you encounter the issue this pull request is solving btw.? Which server was it? Majority of servers are decoding gzip transparently and automatically.

In my particular case i'm uploading huge json payloads, my mobile device works as offline first, so it could be that the next time the device has internet it needs to post huge json changes, but this json can be hugely compressed by using gzip as the values can repeat(mostly embedded devices voltages data that repeat a lot). Would be awesome if Yii2 and Yii3 could support decoding that on application level.

@rob006
Copy link
Contributor

rob006 commented Aug 12, 2025

Why this option is called gzip? According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Encoding#directives there could be other compression algorithms used (including scenarios where multiple algorithms are combined).

@samdark
Copy link
Member

samdark commented Aug 12, 2025

@lucaswitch
Copy link
Author

lucaswitch commented Aug 12, 2025

Why this option is called gzip? According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Encoding#directives there could be other compression algorithms used (including scenarios where multiple algorithms are combined).

Just by convention, since gzip/deflate are the most commonly used, but perhaps a better name would be $zLibDecode or $contentEncodingDecompress , although it’s less familiar to new users.
Also it does not handle all the possible algorithms only two most used ones.

Also i would not support a compressing algorithm that php don't support.

@lucaswitch
Copy link
Author

Why this option is called gzip? According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Encoding#directives there could be other compression algorithms used (including scenarios where multiple algorithms are combined).

Just by convention, since gzip/deflate are the most commonly used, but perhaps a better name would be $zLibDecode or $contentEncodingDecompress , although it’s less familiar to new users. Also it does not handle all the possible algorithms only two most used ones.

Maybe the naming would be better as:

$request = new Request(['useContentEncoding' => true]);

@samdark
Copy link
Member

samdark commented Aug 13, 2025

How about decodeContent?

@rob006
Copy link
Contributor

rob006 commented Aug 13, 2025

How about creating something like $rawBodyDecoder that will store component/closure responsible for decoding raw body? Right now this solution handles really rare use case and it is very limited (it handles only 2 algorithms and adding more will require additional changes in framework). With separate component this could be easily extended by extensions or framework users, and cover more use cases (like encryption for example).

@terabytesoftw
Copy link
Member

according to @rob006 a separate component would allow to solve the task completely.

@lucaswitch
Copy link
Author

How about creating something like $rawBodyDecoder that will store component/closure responsible for decoding raw body? Right now this solution handles really rare use case and it is very limited (it handles only 2 algorithms and adding more will require additional changes in framework). With separate component this could be easily extended by extensions or framework users, and cover more use cases (like encryption for example).

How about creating something like $rawBodyDecoder that will store component/closure responsible for decoding raw body? Right now this solution handles really rare use case and it is very limited (it handles only 2 algorithms and adding more will require additional changes in framework). With separate component this could be easily extended by extensions or framework users, and cover more use cases (like encryption for example).

Agreed! I guess we can use something like a new RequestRawBodyDecoderInterface that would work just like RequestParserInterface.

in web.php

  [
     // ... web
    'request'=> [
     'rawBodyDecoders' => [
         'gzip' => GZipDecoder::class, // we can have that by default cause it's supported by php zlib by default
         'deflate' => DeflateDecoder::class, // we can have that by default cause it's supported by php zlib by default
         'br' => function ($rawBody) { // callable a new class extends RequestDecoderInterface
           return $rawBody; // process it to after parsers deal with it
          }
         // 
     ]
   ]
 ]

in that way we can reuse it outside of http headers scenarios

in some controller

 $uploadedImage = UploadedFile::getInstance($model, 'compressed_image');
 $gzipDecoder = new GZipDecoder(); 
 $binary = $gzipDecoder->parse(file_get_contents($uploadedImage->tempName));

Should i modify this pull request to add it?

@lucaswitch
Copy link
Author

How about creating something like $rawBodyDecoder that will store component/closure responsible for decoding raw body? Right now this solution handles really rare use case and it is very limited (it handles only 2 algorithms and adding more will require additional changes in framework). With separate component this could be easily extended by extensions or framework users, and cover more use cases (like encryption for example).

How about creating something like $rawBodyDecoder that will store component/closure responsible for decoding raw body? Right now this solution handles really rare use case and it is very limited (it handles only 2 algorithms and adding more will require additional changes in framework). With separate component this could be easily extended by extensions or framework users, and cover more use cases (like encryption for example).

Agreed! I guess we can use something like a new RequestRawBodyDecoderInterface that would work just like RequestParserInterface.

in web.php

  [
     // ... web
    'request'=> [
     'rawBodyDecoders' => [
         'gzip' => GZipDecoder::class, // we can have that by default cause it's supported by php zlib by default
         'deflate' => DeflateDecoder::class, // we can have that by default cause it's supported by php zlib by default
         'br' => function ($rawBody) { // callable a new class extends RequestDecoderInterface
           return $rawBody; // process it to after parsers deal with it
          }
         // 
     ]
   ]
 ]

in that way we can reuse it outside of http headers scenarios

in some controller

 $uploadedImage = UploadedFile::getInstance($model, 'compressed_image');
 $gzipDecoder = new GZipDecoder(); 
 $binary = $gzipDecoder->parse(file_get_contents($uploadedImage->tempName));

Should i modify this pull request to add it?

Today we can handle a single request like following, or every request by extending the request class

// manually check the headers then
gzdecode(Yii::$app->request->getRawBody());

@rob006
Copy link
Contributor

rob006 commented Aug 13, 2025

I was thinking about single component that would take headers and raw body and handle everything internally. It would not be Request responsibility to parse headers and call correct decoder - decoder would need to handle it on its own.

@lucaswitch
Copy link
Author

I was thinking about single component that would take headers and raw body and handle everything internally. It would not be Request responsibility to parse headers and call correct decoder - decoder would need to handle it on its own.

Got it.

Copy link
Member

@Arhell Arhell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants