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

Provide a replacement for injecting HttpServletRequest #4125

Closed
gsmet opened this issue Sep 20, 2019 · 39 comments · Fixed by #5581
Closed

Provide a replacement for injecting HttpServletRequest #4125

gsmet opened this issue Sep 20, 2019 · 39 comments · Fixed by #5581
Assignees
Labels
kind/enhancement New feature or request
Milestone

Comments

@gsmet
Copy link
Member

gsmet commented Sep 20, 2019

When using RESTEasy without Undertow, we can't inject HttpServletRequest so we need a replacement for it (and all the other servlet related injections).

A good example of that is the REST JSON quickstart: https://github.com/quarkusio/quarkus-quickstarts/blob/master/rest-json/src/main/java/org/acme/rest/json/LoggingFilter.java .

I think we need something for 0.24.0 as adding Undertow in the quickstart won't really help the case of RESTEasy without Undertow as it's our basic example of a REST service.

/cc @patriot1burke @stuartwdouglas

@gsmet gsmet added the kind/enhancement New feature or request label Sep 20, 2019
@gsmet gsmet added this to the 0.24.0 milestone Sep 20, 2019
@FroMage
Copy link
Member

FroMage commented Oct 2, 2019

I can do that @gsmet . You want an implementation of HttpServletRequest or a new interface with the useful methods?

@FroMage
Copy link
Member

FroMage commented Oct 2, 2019

That interface is huge (https://javaee.github.io/javaee-spec/javadocs/javax/servlet/http/HttpServletRequest.html), so I'd suggest a new interface for the few missing useful methods.

@stuartwdouglas
Copy link
Member

Can't you just inject org.jboss.resteasy.spi.HttpRequest ?

@stuartwdouglas
Copy link
Member

Actually never mind, for some reason I thought that was a spec class and not a resteasy internal one.

@stuartwdouglas
Copy link
Member

javax.ws.rs.core.Request was the one I was thinking of

@FroMage
Copy link
Member

FroMage commented Oct 2, 2019

Yeah, nothing exists ATM. I think we can add it in RESTEasy and later promote it to JAX-RS, because it allows us to implement it for any RESTEasy backend. That'd be very useful. And it would work on resteasy-vertx as well as resteasy-undertow.

@FroMage
Copy link
Member

FroMage commented Oct 2, 2019

javax.ws.rs.core.Request has no interesting method for this.

@FroMage
Copy link
Member

FroMage commented Oct 2, 2019

We could make it a subtype of it, though.

@gsmet
Copy link
Member Author

gsmet commented Oct 2, 2019

@cescoffier changed the filter this morning to use a Vert.x object: https://github.com/quarkusio/quarkus-quickstarts/blob/development/rest-json/src/main/java/org/acme/rest/json/LoggingFilter.java#L21 .

Obviously, this is not portable so I suppose a portable solution in JAX-RS would be better in the long run but, at least for now, we are covered.

@FroMage
Copy link
Member

FroMage commented Oct 2, 2019

So this issue is not on the table for the next milestone anymore, or still?

@FroMage
Copy link
Member

FroMage commented Oct 2, 2019

I think it should still be done, and should be quick to implement.

@FroMage FroMage self-assigned this Oct 2, 2019
@FroMage
Copy link
Member

FroMage commented Oct 3, 2019

Done in resteasy/resteasy#2176 pending two design decisions.

@gsmet gsmet modified the milestones: 0.24.0, 0.25.0 Oct 9, 2019
@cescoffier
Copy link
Member

I fixed it already. Sorry, I forgot to update the issue.

@FroMage
Copy link
Member

FroMage commented Oct 15, 2019

Well, it's not really fixed unless we decide that exposing vert.x is the way to do it. My PR got merged and we're waiting for a release.

@FroMage
Copy link
Member

FroMage commented Oct 24, 2019

Went in RESTEasy 4.4.0.Final which is not released yet.

@gsmet gsmet reopened this Oct 24, 2019
@gsmet
Copy link
Member Author

gsmet commented Oct 24, 2019

@FroMage I'm reopening this one, let's close it once we have upgraded RESTEasy.

@gsmet gsmet removed this from the 0.25.0 milestone Oct 24, 2019
@FroMage
Copy link
Member

FroMage commented Oct 24, 2019

OK, thanks. Perhaps @asoldano has an idea when it will be released?

@asoldano
Copy link
Contributor

@FroMage , I'm targetting end of October for the 4.4.0.Final release

@FroMage
Copy link
Member

FroMage commented Oct 24, 2019

OK, thanks.

@FroMage
Copy link
Member

FroMage commented Oct 29, 2019

I want to document this, but I don't see any documentation on the resteasy vs. servlet deployment in guides. Is this documented somewhere @stuartwdouglas @patriot1burke ?

@patriot1burke
Copy link
Contributor

Inject HttpRequest HttpResponse

@patriot1burke
Copy link
Contributor

org.jboss.resteasy.spi.HttpRequest/HttpResponse

I think what's missing is client IP address

@patriot1burke
Copy link
Contributor

@stuartwdouglas HttpRequest/Response is a public resteasy SPI.

@FroMage
Copy link
Member

FroMage commented Oct 29, 2019

Yeah I added this, I know, but I don't know where best to document it ;)

@gsmet
Copy link
Member Author

gsmet commented Nov 18, 2019

@FroMage for now, please document it at the end of the rest-json guide. Hopefully, we will find a more appropriate place for it once we have a reference guide but let's collect as much as we can for now.

@FroMage
Copy link
Member

FroMage commented Nov 19, 2019

Done: #5581

@gsmet gsmet added this to the 1.0.0.Final milestone Nov 19, 2019
@k0l0ssus
Copy link

k0l0ssus commented Feb 2, 2020

@gsmet I don't understand how the linked PR addresses the OP. It's a documentation update that refers to a component that's supposed to be a substitute, but

  1. The dependency that contains the class isn't part of the BOM nor docs
  2. Isn't actually working - I'm still not able to inject org.jboss.resteasy.plugins.server.sun.http.HttpServerRequest or any of the other children of BaseHttpServerRequest with @Inject. Is there a sample project or unit test I can look at to see how this is supposed to work?

@patriot1burke
Copy link
Contributor

@k0l0ssus I veto (if I actually have one) adding any mock HttpServletRequest into resteasy-without-servlet. If you want to inject an HttpServletRequest, add undertow to your build. The point of resteasy standalone is that there is no servlet overhead and there is a leaner build. With the ability to obtain the IP addresses from HttpRequest, there is no reason to use servlet spec with Resteasy.

@gsmet
Copy link
Member Author

gsmet commented Feb 2, 2020

@k0l0ssus have you looked at the documentation? There's no reference to org.jboss.resteasy.plugins.server.sun.http.HttpServerRequest.

Either you use a standard servlet environment by adding the quarkus-undertow extension or you don't and in this case the replacement is: https://docs.jboss.org/resteasy/docs/4.4.1.Final/javadocs/org/jboss/resteasy/spi/HttpRequest.html .

@k0l0ssus
Copy link

k0l0ssus commented Feb 2, 2020

Apologies @gsmet, I should have clarified I was speaking about https://github.com/resteasy/Resteasy/pull/2176/files as linked through #4125 (comment), not in the OP. Given

  <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-vertx</artifactId>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-undertow</artifactId>
    </dependency>

None of the following injections work:

    @Inject
     //resolves to null but doesn't break startup
    javax.servlet.http.HttpServletRequest injectedHttpServletRequest;  //comes out null;
    @Inject
    //UnsatisfiedResolutionException. Breaks startup
    org.jboss.resteasy.plugins.server.sun.http.HttpServerRequest injectedHttpRequest;  
    @Inject
    //UnsatisfiedResolutionException. Breaks startup
    org.jboss.resteasy.spi.HttpRequest injectedHttpRequest; 
    @Inject
    //Requires JDK11+. UnsatisfiedResolutionException still. Breaks startup
    java.net.http.HttpRequest request;

So as it stands, neither RestEasy nor Undertow provide the object.

@patriot1burke
Copy link
Contributor

patriot1burke commented Feb 2, 2020

@k0l0ssus You have to use @context (with a captial 'C', github is making it lower case) within a JAX-Rs resource not @Inject.

@k0l0ssus
Copy link

k0l0ssus commented Feb 2, 2020

@patriot1burke Thank you! That works.

@38leinaD
Copy link
Contributor

Is there some way to inject the HTTPRequest into a CDI interceptor?
@context does not work; at least for me the member is null

@Context HttpRequest request;

What i want to achive is to annotate certain Jax-RS endpoint with an interceptor-binding and let the CDI-interceptor check some value in the headers.

@alesj
Copy link
Contributor

alesj commented Feb 16, 2020 via email

@38leinaD
Copy link
Contributor

Thanks! That worked. Actually used a JaxRS filter

@Provider
public class AccessControlFilter implements ContainerRequestFilter {

    @Context
    HttpRequest request;
    
    static final ThreadLocal<HttpRequest> HTTP_REQUEST_CONTEXT = new ThreadLocal<HttpRequest>();
    
    @Override
    public void filter(ContainerRequestContext context) {
        HTTP_REQUEST_CONTEXT.set(request);
    }
}

@alesj
Copy link
Contributor

alesj commented Feb 16, 2020 via email

@ShahmirAhmed
Copy link

I've followed this thread and tried the suggestions mentioned, but unfortunately to no avail. I've tried the several approaches that @k0l0ssus used that gave them UnsatisfiedResolutionExceptions with both HttpServletRequest and HttpRequest. I've used @context annotation with HttpServletRequest to which a NullPointerException was thrown, I've also tried using

@RequestScoped
public class RequestDetailsConfig {

  private HttpServletRequest request;
  private ServletContext context;

  public RequestDetailsConfig(@Context HttpServletRequest request, @Context ServletContext context) {
    this.request = request;
    this.context = context;
  }

  public String getRemoteAddress() {
    return request.getRemoteAddr();
  }

  public String getRemoteHost() {
    return request.getRemoteHost();
  }
}

which is a little redundant with the constructor, however, I get back

java.lang.IllegalStateException: UT000048: No request is currently active at io.undertow.servlet.handlers.ServletRequestContext.requireCurrent(ServletRequestContext.java:84)

I'm hoping someone may have some insight, as I've been facing this issue for a few days now. If more context (pun intended) is needed feel free to mention

@FroMage
Copy link
Member

FroMage commented May 26, 2021

Can you open a new issue with a full stack trace and list of extensions please? We need to know what sort of application you're injecting this into.

@jcunliffe1
Copy link

I'm struggling with the same, trying to get any request information injected into the AuthorizationController.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.