ijpiantanida / talkback

A simple HTTP proxy that records and playbacks requests
MIT License
283 stars 41 forks source link

TapeMatchers doesn't consider difference in protocol #78

Closed reith closed 1 year ago

reith commented 1 year ago

Scenario

  1. Make a request to http://github.com and set options.record to talkback.Options.RecordMode.NEW
  2. Make another request to https://github.com and set options.record to talkback.Options.RecordMode.NEW

After 1. Talkback tries to load a tape, but there is no tape, so it creates one after sending the request to the original server. The response is a redirect to https://github.com.

After 2. Talkback loads existing tapes, and matches with tape created for 1. The request itself fails and no other tape will get created.

The root cause seems to be Talkback storing URL's path as URL in the tapes:

{
        headers: { ... },
        url: '/',
        method: 'GET',
        body: '',
}

And the TapeMatcher compares the url part which are same in HTTPS and HTTP cases.

reith commented 1 year ago

Reading the docs again, I guess it was deliberate because of:

Base host url used for this request. Informative, it plays no role during the matching process

I wonder why this behavior is favored.

ijpiantanida commented 1 year ago

Each instance of talkback is proxying a single host, so it should be sending requests to only one of either the http or the https versions.

When you say 1) sends request to http://github.com and 2) sends request to https://github.com, what do you mean exactly?

reith commented 1 year ago

I'm using Talkback in a test suite. I have two tests, one for http://github.com and one for https://github.com. I run a Talkback server for each test, change the URLs of original requests accordingly by setting port number and host name to Talkback server address, and then send the requests. So, in the end I'll have one HTTP request to one Talkback server and one HTTPS request to another server. But the second requests, HTTPS one, will get matched with the tape for the previous HTTP request.

Now, I know I can mitigate the problem if I use separate options.path for tapes made for each test. And it seems to be the only way. I thought I can use urlMatcher but it gets called only if url of page does not match with tape's url.

ijpiantanida commented 1 year ago

Yes, the different servers should be using different tape paths, as each proxied host can be serving different content for the same path - like in the github example, the http host always replies with a redirect to the https one.

reith commented 1 year ago

Yes, the different servers should be using different tape paths, as each proxied host can be serving different content for the same path - like in the github example, the http host always replies with a redirect to the https one.

Great.

Maybe it's good to mention two things in README:

  1. Tapes directory should be separated per host, to prevent matching tapes and requests made to the same URL's path but to different hosts
  2. urlMatcher gets invoked only if the URL's path of a tape and a request matches.