Strip context path correctly if deployed to the root context

[chkal]

VRaptor is currently not stripping the context path correctly if the app is deployed to the root context path.

For ordinary context paths like /myapp it works fine:

/myapp/somepage     ->     /somepath

But for the root context path / it is not correct:

/somepage     ->     somepath

This pull request fixes the problem. I hope I found all the relevant places. :)

[Turini]

Thks @chkal. It sounds good to me. What do you think @LucasC, @garcia-jj?

[garcia-jj]

I use with root context and all paths works fine. Have you tested in which server?

[lucascs]

maybe we should have here a method, on VRaptorRequest/MutableRequest, returning the request uri without the context path, so we don't duplicate this logic here and on DefaultStaticContentHandler. What do you think?

[csokol]

But in that case we would need to cast that HttpServletRequest in DefaultStaticContentHandler, this could be problematic, right? But I agree that we should'nt duplicate this code, maybe we should create a component with this logic and inject it in both classes.

[lucascs]

You're right, not a good solution =/

[lucascs]

big enough for a component?

[csokol]

its kind of overkill :-\

maybe we should leave as it is? create a static method (urgh)?

[lucascs]

I don't like the duplication =/ I prefer the static method.

[csokol]

Fine for me :D
@chkal, what do you think? do you want to refactor that code extracting a static method and reuse it in both classes?

[chkal]

I'm on my phone and on vacation right now, so this will be short...

I had this issue with Undertow, which is the Servlet container of Wildfly.

And yeah, sure, I can refactor the patch. Creating a single static method
for this makes sense. I'll do this next week after returning from vacation.
Am 06.08.2014 21:17 schrieb "csokol" notifications@github.com:

In
vraptor-core/src/main/java/br/com/caelum/vraptor/http/VRaptorRequest.java:

    return uri.replaceFirst("(?i);jsessionid=.*$", "");
}

• private String stripContextPath(String uri) {

•   if ("/".equals(getContextPath())) {
  

•       return uri;
  

•   }
  

•   return uri.substring(getContextPath().length());
  

• }

Fine for me :D
@chkal https://github.com/chkal, what do you think? do you want to
refactor that code extracting a static method and reuse it in both classes?

Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/731/files#r15896613.

[Turini]

I'll do this next week after returning from vacation.

thanks @chkal, enjoy your vacation's last week, we can wait for it :)

[Turini]

I use with root context and all paths works fine. Have you tested in which server?

@garcia-jj, adding this change will break something in your case?
If not, I'll merge this PR locally and extract the behaviour into a new classe.

[garcia-jj]

I can test tonight.
On Aug 21, 2014 2:34 PM, "Rodrigo Turini" notifications@github.com wrote:

I use with root context and all paths works fine. Have you tested in which
server?

@garcia-jj https://github.com/garcia-jj, adding this change will break
something in your case?
If not, I'll merge this PR locally and extract the behaviour into a new
classe.

—
Reply to this email directly or view it on GitHub
#731 (comment).

[chkal]

I refactored the context path removal to a static method called VRaptorRequest.stripContextPath().

I'm not sure if this is the best place for the method. But as we actually would like to have something as VRaptorRequest.getRelativeRequestURI() (which would require ugly casting to use it), it IMHO makes sense to have it in VRaptorRequest.

Any thoughts on this?

[chkal]

Ups. Travis failed. Not sure if this is caused by my commit. I just merged the pull request with master locally and it builds fine.

[Turini]

What do you think about changing to stripContextPath(request)?
So stripContextPath ask for contextPath and requestURI internally.

[Turini]

it IMHO makes sense to have it in VRaptorRequest.

I'm ok with method place too.

[chkal]

Hey @Turini,

yeah, I agree. That's much nicer. I also renamed the method to VRaptorRequest.getRelativeRequestURI(). I like that name as the method basically returns getRequestURI() but relative to the context path.

What do you think?

[garcia-jj]

Sounds good. I like it.

[Turini]

great @chkal, thanks!

[chkal]

You're welcome. And thank YOU for this awesome framework. :)

[garcia-jj]

@Turini, since we are getting some false warnings by Travis, can you run
test tasks on your machine? I'm unable to do this job because I'm on mobile
right now.

Thank you :)

[Turini]

@garcia-jj already done, its working (I'll restart Travis build).

[Turini]

all green now :)