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

Make javax.servlet 2.5 a provided dependency #89

Merged
merged 1 commit into from Sep 5, 2013

Conversation

@jalpedersen
Copy link
Contributor

commented Sep 2, 2013

When running in a servlet 3.0 environment (jetty 8+) we run into the following security exception as the signer for javax.servlet 2.5 and javax.servlet 3.0 differ (at least in the case of Jetty):

Exception in thread "main" java.lang.SecurityException: class "javax.servlet.AsyncContext"'s signer information   does not match signer information of other classes in the same package

By making the dependency provided we use the underlying container's or adapter's version, which is what we want (of course as long javax.servlet remains backwards compatible with 2.5)

@weavejester

This comment has been minimized.

Copy link
Member

commented Sep 2, 2013

Could you ensure the tests pass?

@jalpedersen

This comment has been minimized.

Copy link
Contributor Author

commented Sep 2, 2013

Yes, sorry about that. I forgot to run the tests with the dev profile

@weavejester

This comment has been minimized.

Copy link
Member

commented Sep 3, 2013

Could you tidy up the commit message? The first line of a git commit message is considered to be special, like a subject line. It should be a brief description of the commit, ideally under 70 characters. So perhaps:

Make javax.servlet a provided dependency
This is so we can run in a servlet 3.0 environment as well (Jetty 8+)

Other than that, the commit looks fine.

This is so we can run in a servlet 3.0 environment as well (Jetty 8+)
@jalpedersen

This comment has been minimized.

Copy link
Contributor Author

commented Sep 5, 2013

Commit message has been replaced with your suggestion.

weavejester added a commit that referenced this pull request Sep 5, 2013
Make javax.servlet 2.5 a provided dependency
@weavejester weavejester merged commit 0cf09cc into ring-clojure:master Sep 5, 2013
1 check passed
1 check passed
default The Travis CI build passed
Details
@kul

This comment has been minimized.

Copy link

commented Dec 10, 2013

@weavejester Do you think instead of making dependency provided, the user can exclude it from the mvn, or lein's project.clj for special cases and include again. This affects compojure and other frameworks which are using ring.

@weavejester

This comment has been minimized.

Copy link
Member

commented Dec 10, 2013

Yes, in retrospect this patch may be causing more problems than it solved. On the other hand, it feels like a more "correct" solution not to tie ring-core to a specific servlet-api version. I'm also hesitant to revert a change made in a patch that addresses a legitimate concern.

When I have time to do so, I plan on rewriting the multipart-params middleware to not have a dependency on FileUpload, and therefore servlet-api.

@jarohen

This comment has been minimized.

Copy link

commented Jan 17, 2014

FWIW, +1, this is a real pain. I don't think it's a good idea for projects to declare their dependencies in the README instead of project.clj, and requiring anyone who uses a library to check all of its transitive dependencies manually.

@sunng87

This comment has been minimized.

Copy link

commented Feb 24, 2014

This is a great patch! And it can be even better by removing dependency to servlet API from ring-core totally.

@weavejester , is the multipart-params middleware rewrite in progress or done ? I'm using a non-servlet http server as ring server, but got the ClassNotFoundException issue on servlet classes. It will be great if we can separate ring-core from Servlet API. You know, as the clojure community grows, we might have more non-servlet ring server.

Edit: fix typo and grammar.

@weavejester

This comment has been minimized.

Copy link
Member

commented Feb 24, 2014

The multipart-params rewrite is currently a work in progress, however it's high up my todo list, so I don't expect it to take much longer.

@sunng87

This comment has been minimized.

Copy link

commented May 28, 2014

@weavejester Any updates on the multipart-param rewrite?

@weavejester

This comment has been minimized.

Copy link
Member

commented May 28, 2014

The multipart-params rewrite has slipped down my todo list, since it's not an essential update, and there are a number of other things competing for my attention. I'll add it to Ring 1.4.0, but that'll likely be after the summer.

@laczoka

This comment has been minimized.

Copy link

commented Dec 15, 2014

@weavejester Hi, any update on the multipart-param rewrite? :)

@weavejester

This comment has been minimized.

Copy link
Member

commented Dec 15, 2014

No progress on it since my last comment. It's in a branch, and I still intend for it to be in Ring 1.4.0, but it won't be released this year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.