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

Fixing an instance where http handlers could change the header state of a stored request. #75

Closed
wants to merge 0 commits into from

Conversation

cowboygneox
Copy link
Contributor

Prior to this fix, a request could be stored, and if it were replayed in the same tape session, it would add an X-Betamax header to the stored request. In this case, the tape would be written with these X-Betamax headers in them, and in my particular use case, would result in the occasional parse error.

The YAML should never contain any of the X-Betamax headers when persisted.

@cowboygneox
Copy link
Contributor Author

It would be incredible if we could integrate this into the 1.2-SNAPSHOT in the near future. I would much prefer to use managed dependencies in my projects.

Thanks!

@robfletcher
Copy link
Collaborator

I will definitely merge this in. I'm in the middle of a fairly large refactor, though – hence the delay.

@cowboygneox
Copy link
Contributor Author

@robfletcher This project is of pretty large interest to me. I am interested in becoming a contributor to help jump this project into a more recently updated status. Would you care to enlist my free labor?

@robfletcher
Copy link
Collaborator

Sure. I've just started working on a couple of large issues that will be the core of the next release – #66 and #85. I have just merged the feature branch I've been working on into master. It looks like that means some changes would need to be made to this pull request – probably just because the project has been split into multiple modules. It would be great to get some kind of a test to prove this change works as well. It's not immediately obvious from the change itself why this works where the previous implementation didn't.

I'd definitely like to have more regular contributors to the project – if there are other features you'd be interested in working on let me know.

@cowboygneox
Copy link
Contributor Author

@robfletcher If there is a way to send you a private message on GitHub, I am not aware. Do you want to continue to discuss here?

@robfletcher
Copy link
Collaborator

I don't mind to carry on here but if you'd rather discuss privately you can email me rob@Betamax's maven group

@cowboygneox
Copy link
Contributor Author

@robfletcher Email sent. Let me know if you didn't get it.

@pledbrook
Copy link
Contributor

Ah, so this is probably the issue I've been running into. 👍 for the merge!

@cowboygneox
Copy link
Contributor Author

@pledbrook The repository is currently in a state of flux, but let me know if you need a short-term solution for this.

@robfletcher
Copy link
Collaborator

I'm creating a 1.1.x branch to try to merge in this & @pledbrook's changes for a 1.1.3 release.

@cowboygneox
Copy link
Contributor Author

@robfletcher Awesome :)

@robfletcher
Copy link
Collaborator

Hmm actually it could be tricky. It looks like this change was made against a later version of code than 1.1.2

@cowboygneox
Copy link
Contributor Author

@robfletcher Yeah I don't doubt it. When I wrote this change, I did it against master thinking it could make it into 1.1.3.

@cowboygneox cowboygneox closed this Aug 9, 2013
@cowboygneox cowboygneox reopened this Aug 9, 2013
@cowboygneox
Copy link
Contributor Author

@robfletcher Do you want me to apply this to a 1.1.2 branch?

@robfletcher
Copy link
Collaborator

Ah I think I just need to baseline from a layer revision

On Friday, August 9, 2013, Sean Freitag wrote:

@robfletcher https://github.com/robfletcher Yeah I don't doubt it. When
I wrote this change, I did it against master thinking it could make it into
1.1.3.


Reply to this email directly or view it on GitHubhttps://github.com//pull/75#issuecomment-22392106
.

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