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

Passthrough not working for cross-origin requests #367

Open
ayush987goyal opened this issue Mar 25, 2020 · 9 comments
Open

Passthrough not working for cross-origin requests #367

ayush987goyal opened this issue Mar 25, 2020 · 9 comments

Comments

@ayush987goyal
Copy link

Hi,

I am setting up my routes like so:

new Server({
	routes() {
		this.post(<some-fully-qualified-route>, {...});

		this.post(<another-fully-qualified-route>, {...});

		this.passthrough();
	}
});

This only passes through requests made on same origin but does nothing for the other cross-origin requests. Is this an expected behaviour? If yes, how can we achieve pass-through behaviour for all remaining request except for the one mentioned explicitly in the routes?

@samselikoff
Copy link
Contributor

You can pass through all requests for a single domain like this

this.passthrough('http://api.amazon.com/**')

but unfortunately you can't wildcard all domains. But, passthrough has a function version that you can use to perform whatever check you want:

this.passthrough(request => {
  return request.url.match(/regex/)
})

Let me know if that helps! Here are the full passthrough guides. I know this is a confusing API today, we have plans to improve it so you can passthrough anything that is not defined, and that's very high priority behind some other stuff we're working on now.

@rassie
Copy link

rassie commented May 17, 2020

@samselikoff do you know a way forward on this? If I'm not mistaken there is a rather cumbersome inconsistency: if you define a this.passthrough with a path, it's applied after any non-passthrough definitions, so basically this.passthrough() at the end just works. But the variant of this.passthrough with a function as its parameter gets applied before any other Pretender rule, so can't be a catch-all anymore and I'd need to add all previously defined paths to an exclusion list inside this.passthrough. I also cannot reuse Pretender's checkPassthrough, since it's not declared in typings and is also a class method, so that augmenting locally is not possible.

Is there any way we could resolve this? Maybe only evaluate and apply this.passthroughChecks if originalCheckPassthrough is false (here)?

@samselikoff
Copy link
Contributor

Hm. I agree that is inconsistent + unfortunate behavior.

Could you use a this.passthrough() at the end of your routes to take care of all previously defined route handlers, and then another call to passthrough(() => ()) with a function to account for any other URLs?

Our goal here is to eventually add a canHandle method to server, and to make the default passthrough function something like

this.passthrough(request => {
  return !server.canHandle(request)
})

In other words, pass through every request the server doesn't have a defined route handler for.

@rassie
Copy link

rassie commented May 18, 2020

Wouldn't canHandle be a very slim wrapper around checkPassthrough?

@samselikoff
Copy link
Contributor

No, it would use route recognizer to see if a runtime request to a URL like /api/users/1 was able to be handled by a Mirage server with a route handler like this.get('/api/users/:id') defined.

@samselikoff
Copy link
Contributor

We started work for it here: #320

We paused work on it because we needed to focus on the tutorial + existing bugs.

@rassie
Copy link

rassie commented May 18, 2020

No, it would use route recognizer to see if a runtime request to a URL like /api/users/1 was able to be handled by a Mirage server with a route handler like this.get('/api/users/:id') defined.

I'm sorry for sounding like a broken record (just want to make sure I understand fully), but isn't that precisely what checkPassthrough is doing?

[...]
  checkPassthrough: function checkPassthrough(request) {
    let verb = request.method.toUpperCase();
    let path = parseURL(request.url).fullpath;
    let recognized = this.hosts.forURL(request.url)[verb].recognize(path);
    let match = recognized && recognized[0];

    if ((match && match.handler === PASSTHROUGH) || this.forcePassthrough)  {
      this.passthroughRequests.push(request);
      this.passthroughRequest(verb, path, request);
      return true;
    }

    return false;
  }
[...]

@samselikoff
Copy link
Contributor

No worries, the questions don't bother me! I haven't looked at this code in a while and am just trying to answer questions at a higher level to explain our plans, so I could be wrong. But I think checkPassthrough looks for both recognized and that the handler is PASSTHROUGH.

In our case we want a method server.canHandle(request) that returns true if a normal (non-passthrough) route is defined. So basically, we want to know if a URL is recognized.

Then Mirage's this.passthrough method (which I think runs first) can check whether the URL is recognized, and if not, go ahead and let the request pass through.

@ryanto
Copy link
Contributor

ryanto commented May 20, 2020

Hi @rassie

Maybe only evaluate and apply this.passthroughChecks if originalCheckPassthrough is false?

I think this seems reasonable, but I'm having a hard time visualizing a config that highlights this change. Could you share your server's routes and examples of requests you'd like mirage to handle vs passthrough. That'll help!

I think getting a test case of sorts in place is the best way to see what's going on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants