Skip to content

Cancel MITM request context when its handling finishes#634

Merged
ErikPelli merged 3 commits intomasterfrom
fix-request-context
Jan 21, 2025
Merged

Cancel MITM request context when its handling finishes#634
ErikPelli merged 3 commits intomasterfrom
fix-request-context

Conversation

@ErikPelli
Copy link
Copy Markdown
Collaborator

Fixes #633.
Add Go context cancel to a custom parsed request, since we handle it.

@ErikPelli
Copy link
Copy Markdown
Collaborator Author

@alessiodallapiazza I tested your example script with this release and it works perfectly.
If it's all good for you, I think I can merge this pull request as a stable release, since we're also passing the test pipeline without issues.
What do you think?

@alessiodallapiazza
Copy link
Copy Markdown
Contributor

I’ve run some tests as well, and everything looks good on my end. 👍

@ErikPelli ErikPelli merged commit aa6e8d3 into master Jan 21, 2025
@ErikPelli ErikPelli deleted the fix-request-context branch January 21, 2025 10:29
@thisislawatts
Copy link
Copy Markdown

@ErikPelli why are automated test cases not being introduced to validate the behaviour modifications that are being made here?

@ErikPelli
Copy link
Copy Markdown
Collaborator Author

I don't think we need to introduce new tests for every single change we made, do you think they are necessary for this one?

@thisislawatts
Copy link
Copy Markdown

thisislawatts commented Jan 28, 2025

This specific change was made to address a specific issue #633. So I guess if it was a significant enough bug to warrant a change it also seems worth capturing the desired behaviour in an automated test. Otherwise, I am curious how you would avoid introducing regressions or reoccurrences of the same bug?

@ErikPelli
Copy link
Copy Markdown
Collaborator Author

Added the test in another pull request

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.

Context Done Not Propagated in Goproxy Extension Limiter During HTTPS/CONNECT

3 participants