Skip to content
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.

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

Fix a Webmock regression #159

Merged
merged 2 commits into from
Feb 5, 2018
Merged

Conversation

julik
Copy link
Collaborator

@julik julik commented Feb 3, 2018

Webmock overrides a ton of the Patron session internals, and it furnishes a modified
string of headers to the internal parse_headers method. That string, unlike real libCURL
header strings, does not have the terminating CRLF at the end of the last header. Nor does
it have an empty line at the very end ending with CRLF. This led our parser to return early
since it could not find the string terminator, and the early return in the header parser
code did not return the responses array that was already prepared for use. Even though
this is not completely "clean" we will just use a simpler parsing approach without scans.

That said, the expectation that Webmock will keep working with the refactors we appply -
given how much private stuff they rely on - might be a little naive.

Closes #157

Webmock overrides a ton of the Patron session internals, and it furnishes a modified
string of headers to the internal parse_headers method. That string, unlike real libCURL
header strings, does not have the terminating CRLF at the end of the last header. Nor does
it have an empty line at the very end ending with CRLF. This led our parser to return early
since it could not find the string terminator, and the early return in the header parser
code did not return the `responses` array that was already prepared for use. Even though
this is not completely "clean" we will just use a simpler parsing approach without scans.

That said, the expectation that Webmock will keep working with the refactors we appply -
given how much private stuff they rely on - might be a little naive.

Closes #157
@julik julik requested a review from toland February 3, 2018 21:37
Copy link
Owner

@toland toland left a comment

Choose a reason for hiding this comment

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

👍

@julik julik merged commit b146c3f into master Feb 5, 2018
@toland toland deleted the parse-headers-without-trailing-crlf branch February 23, 2018 16:34
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.

2 participants