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

Add HTTPServerRequest.routerRootDir property #1301

Merged
merged 3 commits into from
Nov 9, 2015

Conversation

Yoplitein
Copy link
Contributor

This allows for emitting the correct path when using a URLRouter with a prefix.

Consider the index of a router with prefix "/a/deep/path" which renders:

a(href="#{req.rootDir}") rootDir
a(href="#{req.routerRootDir}") routerRootDir

The following is output:

<a href="../../../">rootDir</a>
<a href="../../../a/deep/path/">routerRootDir</a>

@s-ludwig
Copy link
Member

This idea sounds useful, I worked around this by hand for some projects already. For new projects , something similar for registerWebInterface would be similarly useful.

I see a two issues with the concrete approach:

  • This obviously only works for a single router, but routers could be nested. Probably not the common case, but it exists. A solution would be to update the request when an actual route match was found instead of once at the beginning.
  • The router is a higher level concept that is layered on top. The low-level HTTP API shouldn't be affected by it. What I'd propose is to simply put the relative path into HTTPServerRequest.params["routerRootDir"] instead. For computing the relative path, it's probably a good idea to use Path.relativeTo, to get a shorter path.

@Yoplitein
Copy link
Contributor Author

I'm trying to use Path, but I can't figure out how to wrangle it into submission.
The following was the only way I could get it to actually do anything:

Path path = Path([PathEntry("a"), PathEntry("deep"), PathEntry("path")], true);
Path root = Path([PathEntry(".."), PathEntry(".."), PathEntry("..")], true);
string routerRootDir = path.relativeTo(root).toString;

Everything else causes the assert at path.d:145 to fail.
And it still emits ../../../a/deep/path.

How do I properly go about this?

@s-ludwig
Copy link
Member

It should be Path(router.prefix).relativeTo(Path(req.path)).toString() - assuming that both, router.prefix and req.path, start with a "/", which should always be true.

@Yoplitein
Copy link
Contributor Author

That doesn't seem to be quite right either.

For paths without a trailing slash, it leaves off the topmost path component. E.g. /a/deep/path/param => /a/deep/
And at the index of the router, it is equivalent to req.rootDir.

Edit: I should mention this is how the browser handles it.

@s-ludwig
Copy link
Member

Hm right, forgot about that, this issue just came up somewhere else recently. Trailing slash handling doesn't quite match the rules needed here. This should fix it:

auto rp = Path(req.path);
if (!rp.endsWithSlash) rp = rp[0 .. $-1];
string result = Path(prefix).relativeTo(req.path).toString();

@Yoplitein
Copy link
Contributor Author

Ok, I think that's all the edge cases ironed out. Thanks for the help.

@s-ludwig
Copy link
Member

s-ludwig commented Nov 9, 2015

Thanks, looks good.

s-ludwig added a commit that referenced this pull request Nov 9, 2015
Add HTTPServerRequest.routerRootDir property
@s-ludwig s-ludwig merged commit 55dc9f8 into vibe-d:master Nov 9, 2015
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.

2 participants