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

(upstream) toxic for http headers #215

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

worldtiki
Copy link

Raising this pr and hoping to get some traction on Toxiproxy again :)

Context: it's common to use the host header to direct a http request to a particular service, specially when using load balancers/reverse proxies. This is a problem with toxiproxy as the victim app will be configured to send the requests to localhost:port and when toxiproxy forwards it to the reverse proxy the host header will be localhost and the request will fail to be delivered to the correct service.

This toxic is an upstream toxic only and allows forcing http headers for a proxy (host and others). I understand there's been some discussion about bidirectional toxics but that work was not done yet, and I think it would be of interest to solve this issue now and refactor this solution later to add downstream support

@xthexder
Copy link
Contributor

Actually, I think most of the work on bidirectional toxics was done, it just needed some review and polishing. I got stalled and stopped working on it, but I could put the time in if there's interest.

I'd love for toxiproxy to start supporting protocol aware toxics, since I think it's a powerful tool, and would have a lot of applications.

@maxgalbu
Copy link

maxgalbu commented Nov 20, 2018

Just FYI for people looking here for a HTTP toxic, I found a different way of delaying HTTP requests and returning errors by using https://www.mocky.io/, you can specify a HTTP status code and you can pass ?mocky-delay to the url to have a delay in the response

@nikos912000
Copy link

Hey Toxiproxy team,
Is there any update on this? We're using Toxiproxy a lot but without forwarding the host header our requests fail to get forwarder to the right service.

@jpittis
Copy link
Contributor

jpittis commented Apr 25, 2019

Regarding bidirection toxics, as @xthexder explained the work is mostly done. Someone needs to own understanding + polishing + shipping it. Currently no one is owning that work though. (#132)

Regarding this PR, I don't see the issue with adding a "hackier" short term solution in Toxic form. Especially since toxics are well de-coupled from the rest of the system. We should get this PR reviewed.

@ghost ghost added the cla-needed label Nov 7, 2019
@ghost ghost removed the cla-needed label Jan 12, 2021
@worldtiki
Copy link
Author

Hi folks! Is there still interest in this?

@avaakash
Copy link

Hi @worldtiki, I was trying out this toxic, but not able to pass headers as an attribute, there is a problem in unmarshalling. I wanted to know how were you creating this toxic using the CLI.
Command I used in CLI
./toxiproxy-cli create -l localhost:2002 -u wttr.in:80 test && ./toxiproxy-cli toxic add -t response_header -a headers={"new":"test"} test

Error I am getting
Failed to add toxic: failed to add toxic to proxy test: AddToxic: HTTP 400: bad request body: json: cannot unmarshal string into Go struct field HeaderToxic.attributes.headers of type map[string]string

@dianadevasia dianadevasia deleted the branch Shopify:main April 20, 2023 16:13
@worldtiki
Copy link
Author

@dianadevasia ?

@dianadevasia
Copy link
Contributor

@worldtiki : Sorry the PR got accidentally closed when the branch got renamed. I have reopened the PR and updated the source.

@miry miry removed their assignment May 2, 2023
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.

8 participants