Skip to content

apc.php / gzdecode rss - #601

Closed
SailorMax wants to merge 1 commit into
krakjoe:masterfrom
SailorMax:patch-1
Closed

apc.php / gzdecode rss#601
SailorMax wants to merge 1 commit into
krakjoe:masterfrom
SailorMax:patch-1

Conversation

@SailorMax

Copy link
Copy Markdown

decode rss content if it compressed

decode rss content if it compressed

@madmajestro madmajestro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not a maintainer, but quite familiar with the code.

It doesn't feel right to implement something like this, but since the server seems to ignore the Accept-Encoding: identity header, I don't see a better solution.

If anyone has a better idea / more elegant approach, please let me know.

Closes #600

Comment thread apc.php
Comment on lines +1120 to +1122
if ($rss[0] !== '<') {
$rss = gzdecode($rss);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if ($rss[0] !== '<') {
$rss = gzdecode($rss);
}
if (ord($rss[0]) === 0x1f && ord($rss[1]) === 0x8b) {
$rss = gzdecode($rss);
}
  • I propose to check for the magic numbers 0x1f and 0x8b to identify the data as a gzip stream. Imho, this is more precise and it allows us to pick different decompression algorithms if necessary someday.
  • A decompression failure (if gzdecode() returns false) should be handled properly. Perhaps it makes sense to do this a few lines earlier (before line 1115)?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agree that it's better to check the magic numbers. (Even better would probably to check for the content-encoding response header.)

@madmajestro

Copy link
Copy Markdown
Collaborator

And just a suggestion to adapt the commit title: Decode gzip-compressed rss content in apc.php
This is more in line with the style used in the other commits.

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.

3 participants