Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

FIX - Put back the insensitive replacements for Transfer-Encoding and Content-Encoding. #10

Closed
wants to merge 1 commit into from

Conversation

nbochenko
Copy link

No description provided.

@nbochenko
Copy link
Author

@blanchonvincent

@Martin-P
Copy link
Contributor

@nbochenko Perhaps you can add some text to this PR and:

  • explain what bug you encountered
  • create unit tests to reproduce the issue
  • explain why your code change is a fix for your issue

@weierophinney
Copy link
Member

@nbochenko Ping - Please see the comments from @Martin-P ; we need more detail, specifically in the form of unit tests, to ensure that the changes are correct, and that we don't accidentally revert them in the future.

@Maks3w
Copy link
Member

Maks3w commented Oct 21, 2015

Closed due inactivity

@Maks3w Maks3w closed this Oct 21, 2015
@wiese
Copy link

wiese commented Oct 21, 2015

I get your point wanting a test for this, so we don't break it again in the future, but this fixes a real bug introduced during nbochenko@ddf5a83, which noboby had any issue merging into the main line without a test - so some initiative from the core team and the original author should be just as expected as from someone who, at least, found the bug and was kind enough to report it. I'll try and contribute some coverage here rather sooner than later.

@Martin-P
Copy link
Contributor

Ping @Maks3w @weierophinney.
I can confirm what @wiese says. @kasitmp never added any tests to the original PR (zendframework/zendframework#6691) and @weierophinney just merged the PR without tests.

Before zendframework/zendframework#6691 there was stri_replace (watch the i), but that PR removed the case-insensitive matching and made it case-sensitive.

Perhaps @kasitmp or @weierophinney can add the proper tests for this as they caused this issue 😉

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

Successfully merging this pull request may close these issues.

5 participants