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

AwsProxyServletContext Does Not Support Servlet Filter Registration #15

Closed
blakeney opened this issue Mar 7, 2017 · 9 comments
Closed
Assignees
Milestone

Comments

@blakeney
Copy link

blakeney commented Mar 7, 2017

Hello,

I am trying to use this library along with Spring Boot to create a REST application that uses javax.servlet.Filter implementations to transform responses. I see that all of the filter-related methods in AwsProxyServletContext are overridden to simply return null or to do nothing (source). This would seem to indicate that Filters are not supported with this container. Is that correct?

I have attempted several methods of registering Filters, which work properly with my local tests, but do not seem to have any effect when running in a live Lambda.

If I cannot use Filter, I will try to use Spring's HandlerInterceptor mechanism instead.

@blakeney blakeney changed the title Servlet Filter Registration? AwsProxyServletContext Does Not Support Servlet Filter Registration Mar 8, 2017
@blakeney
Copy link
Author

blakeney commented Mar 8, 2017

I did some more investigation and determined with more certainty that indeed, the current implementation does not support Filter registration (or Servlet registration, for that matter). I also determined that HandlerInterceptors will not serve my particular need, so I am just going to work around this in the handleRequest method of my own project for the time being, essentially bypassing Spring for part of my code.

I probably will not have time to patch this library to add Filter support myself any time soon, so unfortunately I can't offer a Pull Request at the moment. I'll leave the ticket open for now in case anyone else is interested in this as a feature, but I'm also okay with it if this is a "Won't Fix." Thanks!

@sapessi
Copy link
Collaborator

sapessi commented Mar 8, 2017

Correct, at the moment it does not. I'd be interested to take a look at your filter code and see if/how we can add support for it. Anything you can share on GitHub or here?

@sapessi sapessi self-assigned this Mar 8, 2017
@AlainJanssens
Copy link

Is this also the reason why '@Valid' isn't working?

@sapessi
Copy link
Collaborator

sapessi commented Mar 13, 2017

I'm no spring expert but doing some research I can see that the @Valid annotation does run as a filter. We are currently designing filters support based on this issue and we'll have something in a branch over the next few weeks.

@AlainJanssens
Copy link

Thanks for the answer @sapessi ..

I created a separated issue for this...

@sapessi
Copy link
Collaborator

sapessi commented Mar 13, 2017

I'll keep that issue open until I can confirm for certain that this is the cause of the issue. If it is, then I'll close the separate issue and group everything in here. Otherwise we'll address it there.

@sapessi
Copy link
Collaborator

sapessi commented Mar 22, 2017

Hey @blakeney, could you share some sample code on how you initialize (and add) filters in your spring application?

@sapessi
Copy link
Collaborator

sapessi commented Mar 23, 2017

I've pushed a first version of the filter support to the servlet-improvements branch. This is the commit d7bec0a. You can register filters with the setStartupHandler lambda:

try {
    handler = SpringLambdaContainerHandler.getAwsProxyHandler(EchoSpringAppConfig.class);
    handler.setStartupHandler(c -> {
        FilterRegistration.Dynamic registration = c.addFilter("CustomHeaderFilter", CustomHeaderFilter.class);
        // update the registration to map to a path
        registration.addMappingForUrlPatterns(EnumSet.of(DispatcherType.REQUEST), true, "/*");
        // servlet name mappings are disabled and will throw an exception
    });
} catch (ContainerInitializationException e) {
    e.printStackTrace();
}

Next I will work on supporting the filter annotations.

sapessi added a commit that referenced this issue Mar 24, 2017
Bug fixes and additional unit tests. Annotation support completes the
feature required to address #15. The service does not automatically
load annotated filter classes, these will have to be added manually
with the new startup event.
sapessi added a commit that referenced this issue Mar 28, 2017
More unit tests for #15 and added a unit test to verify #16.
Updated readme to include servlet filter section
@sapessi sapessi added this to the Release 0.4 milestone Mar 31, 2017
@sapessi
Copy link
Collaborator

sapessi commented Apr 13, 2017

Version 0.4 is out and adds support for servlet filter:

handler.setStartupHandler(c -> {
    FilterRegistration.Dynamic registration = c.addFilter("CustomHeaderFilter", CustomHeaderFilter.class);
    // update the registration to map to a path
    registration.addMappingForUrlPatterns(EnumSet.of(DispatcherType.REQUEST), true, "/*");
    // servlet name mappings are disabled and will throw an exception
});

Closing this issue.

@sapessi sapessi closed this as completed Apr 13, 2017
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

3 participants