You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "dsmiley (via GitHub)" <gi...@apache.org> on 2026/03/27 02:15:42 UTC
[PR] SOLR-18168: Remove path exclusion filter [solr]
dsmiley opened a new pull request, #4243:
URL: https://github.com/apache/solr/pull/4243
Now that SolrServlet exists, we are unblocked from removing PathExclusionFilter. Instead, its task can be accomplished with web.xml configuration. Furthermore, admin UI serving should be exclusively tested with BATS (integration) – JettySolrRunner can be simpler.
[SOLR-18168](https://issues.apache.org/jira/browse/SOLR-18168)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18168: Remove path exclusion filter [solr]
Posted by "gus-asf (via GitHub)" <gi...@apache.org>.
gus-asf commented on code in PR #4243:
URL: https://github.com/apache/solr/pull/4243#discussion_r3006803866
##########
solr/webapp/web/WEB-INF/web.xml:
##########
@@ -24,81 +24,95 @@
<listener>
<listener-class>org.apache.solr.servlet.CoreContainerProvider</listener-class>
</listener>
- <!-- Any path (name) registered in solrconfig.xml will be sent to that filter -->
- <filter>
- <filter-name>PathExclusionsFilter</filter-name>
- <filter-class>org.apache.solr.servlet.PathExclusionFilter</filter-class>
- <!--
- Exclude patterns is a list of directories that would be short-circuited by this
- Filter. It includes all Admin UI related static content.
- NOTE: It is NOT a pattern but only matches the start of the HTTP ServletPath.
- -->
- <init-param>
- <param-name>excludePatterns</param-name>
- <param-value>/partials/.+,/libs/.+,/css/.+,/js/.+,/img/.+,/templates/.+,/ui/.*</param-value>
- </init-param>
- </filter>
-
- <filter-mapping>
- <filter-name>PathExclusionsFilter</filter-name>
- <url-pattern>/*</url-pattern>
- </filter-mapping>
<filter>
<filter-name>SolrFilter</filter-name>
<filter-class>org.apache.solr.servlet.RequiredSolrRequestFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>SolrFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<filter>
<filter-name>RateLimitFilter</filter-name>
<filter-class>org.apache.solr.servlet.RateLimitFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>RateLimitFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<filter>
<filter-name>TracingFilter</filter-name>
<filter-class>org.apache.solr.servlet.TracingFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>TracingFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<filter>
<filter-name>AuthenticationFilter</filter-name>
<filter-class>org.apache.solr.servlet.AuthenticationFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>AuthenticationFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<servlet>
- <servlet-name>SolrServlet</servlet-name>
+ <servlet-name>default</servlet-name>
Review Comment:
"default" seems to communicate nothing useful and also how does that interact with the jetty notion of the default servlet. Maybe it would be clearer to call it "Solr" or "Search" ? SolrServlet was fine too.. though I can see wanting to clarify that it is the name given here, not the name of the class that is being bound.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18168: Remove path exclusion filter [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #4243:
URL: https://github.com/apache/solr/pull/4243#issuecomment-4178737189
Sadly the "Solr Script Tests" (iTest/bats) are flaky, at least when run via GHA. They passed locally. So I'm gonna merge this very soon.
I'll throw up a separate PR to help us debug BATS tests in CI. I don't like the GHA script name "Solr Script Tests". We colloquially call these "BATS" so lets name it how we call it!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18168: Remove path exclusion filter [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4243:
URL: https://github.com/apache/solr/pull/4243#discussion_r2998634917
##########
solr/packaging/test/test_adminconsole_urls.bats:
##########
Review Comment:
more testing here to make up for the removal elsewhere. I was sure to check a random CSS file too as it's served via different routing path.
##########
changelog/unreleased/SOLR-18041.yml:
##########
Review Comment:
The filter came and went inside a release. So instead I'm communicating the observed sum of the two.
##########
solr/packaging/build.gradle:
##########
Review Comment:
The Gradle up-to-date tracking wasn't working so I fixed it. CC @epugh as I suspect you run these tests more than any of us.
##########
solr/webapp/web/WEB-INF/web.xml:
##########
@@ -24,81 +24,95 @@
<listener>
<listener-class>org.apache.solr.servlet.CoreContainerProvider</listener-class>
</listener>
- <!-- Any path (name) registered in solrconfig.xml will be sent to that filter -->
- <filter>
- <filter-name>PathExclusionsFilter</filter-name>
- <filter-class>org.apache.solr.servlet.PathExclusionFilter</filter-class>
- <!--
- Exclude patterns is a list of directories that would be short-circuited by this
- Filter. It includes all Admin UI related static content.
- NOTE: It is NOT a pattern but only matches the start of the HTTP ServletPath.
- -->
- <init-param>
- <param-name>excludePatterns</param-name>
- <param-value>/partials/.+,/libs/.+,/css/.+,/js/.+,/img/.+,/templates/.+,/ui/.*</param-value>
- </init-param>
- </filter>
-
- <filter-mapping>
- <filter-name>PathExclusionsFilter</filter-name>
- <url-pattern>/*</url-pattern>
- </filter-mapping>
<filter>
<filter-name>SolrFilter</filter-name>
<filter-class>org.apache.solr.servlet.RequiredSolrRequestFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>SolrFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<filter>
<filter-name>RateLimitFilter</filter-name>
<filter-class>org.apache.solr.servlet.RateLimitFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>RateLimitFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<filter>
<filter-name>TracingFilter</filter-name>
<filter-class>org.apache.solr.servlet.TracingFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>TracingFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<filter>
<filter-name>AuthenticationFilter</filter-name>
<filter-class>org.apache.solr.servlet.AuthenticationFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>AuthenticationFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<servlet>
- <servlet-name>SolrServlet</servlet-name>
+ <servlet-name>default</servlet-name>
Review Comment:
by changing this to "default", it means we don't need to register it for `/*`
##########
solr/webapp/web/WEB-INF/web.xml:
##########
@@ -24,81 +24,95 @@
<listener>
<listener-class>org.apache.solr.servlet.CoreContainerProvider</listener-class>
</listener>
- <!-- Any path (name) registered in solrconfig.xml will be sent to that filter -->
- <filter>
- <filter-name>PathExclusionsFilter</filter-name>
- <filter-class>org.apache.solr.servlet.PathExclusionFilter</filter-class>
- <!--
- Exclude patterns is a list of directories that would be short-circuited by this
- Filter. It includes all Admin UI related static content.
- NOTE: It is NOT a pattern but only matches the start of the HTTP ServletPath.
- -->
- <init-param>
- <param-name>excludePatterns</param-name>
- <param-value>/partials/.+,/libs/.+,/css/.+,/js/.+,/img/.+,/templates/.+,/ui/.*</param-value>
- </init-param>
- </filter>
-
- <filter-mapping>
- <filter-name>PathExclusionsFilter</filter-name>
- <url-pattern>/*</url-pattern>
- </filter-mapping>
<filter>
<filter-name>SolrFilter</filter-name>
<filter-class>org.apache.solr.servlet.RequiredSolrRequestFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>SolrFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
Review Comment:
referencing by name instead of URL pattern.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18168: Remove path exclusion filter [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #4243:
URL: https://github.com/apache/solr/pull/4243#discussion_r3000375765
##########
solr/packaging/build.gradle:
##########
@@ -282,6 +282,7 @@ task integrationTests(type: BatsTask) {
}
inputs.dir(distDir)
+ inputs.dir('test') // Track .bats test files and helper as inputs
Review Comment:
Thanks for this, there are times when I get weirdness, and maybe this fixes that!
##########
solr/packaging/test/test_adminconsole_urls.bats:
##########
@@ -28,11 +28,46 @@ teardown() {
solr stop --all >/dev/null 2>&1
}
+
@test "assert able to launch solr admin console" {
run solr start
+ # Check HTTP status code
run curl -s -o /dev/null -w "%{http_code}" http://localhost:${SOLR_PORT}/solr/
+ assert_output "200"
+
+ # Check Content-Type header is text/html
+ local content_type=$(curl -s -I http://localhost:${SOLR_PORT}/solr/ | grep -i "Content-Type:" | tr -d '\r')
Review Comment:
Can this be a run and an assert include? Just to be more bats like and less fancy bass?
##########
solr/packaging/test/test_adminconsole_urls.bats:
##########
@@ -28,11 +28,46 @@ teardown() {
solr stop --all >/dev/null 2>&1
}
+
@test "assert able to launch solr admin console" {
run solr start
+ # Check HTTP status code
run curl -s -o /dev/null -w "%{http_code}" http://localhost:${SOLR_PORT}/solr/
+ assert_output "200"
+
+ # Check Content-Type header is text/html
+ local content_type=$(curl -s -I http://localhost:${SOLR_PORT}/solr/ | grep -i "Content-Type:" | tr -d '\r')
+ [[ "$content_type" == *"text/html"* ]]
- # Check if the HTTP response code is 200 (OK)
+ # Check that response body contains HTML
+ local response_body=$(curl -s http://localhost:${SOLR_PORT}/solr/)
+ [[ "$response_body" == *"<html"* ]] || [[ "$response_body" == *"<!DOCTYPE"* ]]
+
+ # Check that a CSS file returns 200
+ run curl -s -o /dev/null -w "%{http_code}" http://localhost:${SOLR_PORT}/solr/css/angular/chosen.css
assert_output "200"
+
+ # Check Content-Type header is text/css
+ local content_type=$(curl -s -I http://localhost:${SOLR_PORT}/solr/css/angular/chosen.css | grep -i "Content-Type:" | tr -d '\r')
+ [[ "$content_type" == *"text/css"* ]]
+
+ # Check that response body contains actual CSS content
+ local response_body=$(curl -s http://localhost:${SOLR_PORT}/solr/css/angular/chosen.css)
+ [[ "$response_body" == *"{"* ]] && [[ "$response_body" == *"}"* ]]
+}
+
+@test "assert CSP header contains custom connect src URLs" {
+ # Set custom CSP connect-src URLs via system property
+ local csp_urls="http://example1.com/token,https://example2.com/path/uri1,http://example3.com/oauth2/uri2"
+
+ run solr start -Dsolr.ui.headers.csp.connect-src.urls="${csp_urls}"
+
+ # Get the Content-Security-Policy header value
+ local csp_header=$(curl -s -I http://localhost:${SOLR_PORT}/solr/ | grep -i "Content-Security-Policy:" | tr -d '\r')
+
+ # Check that the CSP header contains each of our custom URLs
+ [[ "$csp_header" == *"http://example1.com/token"* ]]
Review Comment:
These could use the standard bats? I like keeping to the bats conventions as much as possible to reduce the learning when you read these!
##########
solr/webapp/web/WEB-INF/web.xml:
##########
@@ -24,81 +24,95 @@
<listener>
<listener-class>org.apache.solr.servlet.CoreContainerProvider</listener-class>
</listener>
- <!-- Any path (name) registered in solrconfig.xml will be sent to that filter -->
- <filter>
- <filter-name>PathExclusionsFilter</filter-name>
- <filter-class>org.apache.solr.servlet.PathExclusionFilter</filter-class>
- <!--
- Exclude patterns is a list of directories that would be short-circuited by this
- Filter. It includes all Admin UI related static content.
- NOTE: It is NOT a pattern but only matches the start of the HTTP ServletPath.
- -->
- <init-param>
- <param-name>excludePatterns</param-name>
- <param-value>/partials/.+,/libs/.+,/css/.+,/js/.+,/img/.+,/templates/.+,/ui/.*</param-value>
- </init-param>
- </filter>
-
- <filter-mapping>
- <filter-name>PathExclusionsFilter</filter-name>
- <url-pattern>/*</url-pattern>
- </filter-mapping>
<filter>
<filter-name>SolrFilter</filter-name>
<filter-class>org.apache.solr.servlet.RequiredSolrRequestFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>SolrFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<filter>
<filter-name>RateLimitFilter</filter-name>
<filter-class>org.apache.solr.servlet.RateLimitFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>RateLimitFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<filter>
<filter-name>TracingFilter</filter-name>
<filter-class>org.apache.solr.servlet.TracingFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>TracingFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<filter>
<filter-name>AuthenticationFilter</filter-name>
<filter-class>org.apache.solr.servlet.AuthenticationFilter</filter-class>
</filter>
-
<filter-mapping>
<filter-name>AuthenticationFilter</filter-name>
- <url-pattern>/*</url-pattern>
+ <servlet-name>default</servlet-name>
</filter-mapping>
<servlet>
- <servlet-name>SolrServlet</servlet-name>
+ <servlet-name>default</servlet-name>
Review Comment:
I think I liked the simplicity of the pattern versus needing to know what "default" is, but we should do whatever is the standard approxh for these!
##########
solr/packaging/test/test_adminconsole_urls.bats:
##########
@@ -28,11 +28,46 @@ teardown() {
solr stop --all >/dev/null 2>&1
}
+
@test "assert able to launch solr admin console" {
run solr start
+ # Check HTTP status code
run curl -s -o /dev/null -w "%{http_code}" http://localhost:${SOLR_PORT}/solr/
+ assert_output "200"
+
+ # Check Content-Type header is text/html
+ local content_type=$(curl -s -I http://localhost:${SOLR_PORT}/solr/ | grep -i "Content-Type:" | tr -d '\r')
+ [[ "$content_type" == *"text/html"* ]]
- # Check if the HTTP response code is 200 (OK)
+ # Check that response body contains HTML
+ local response_body=$(curl -s http://localhost:${SOLR_PORT}/solr/)
+ [[ "$response_body" == *"<html"* ]] || [[ "$response_body" == *"<!DOCTYPE"* ]]
Review Comment:
Likewise with the assert partial command?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18168: Remove path exclusion filter [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4243:
URL: https://github.com/apache/solr/pull/4243#discussion_r3006205590
##########
AGENTS.md:
##########
Review Comment:
@epugh I added some info based on your standards for BATS tests.
CC @janhoy as well as there's some other stuff I've added from experience lately
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18168: Remove path exclusion filter [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley merged PR #4243:
URL: https://github.com/apache/solr/pull/4243
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18168: Remove path exclusion filter [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #4243:
URL: https://github.com/apache/solr/pull/4243#issuecomment-4149418241
I have another PR to follow this one that simplifies JettySolrRunner a bit but seems a touch out of scope to include in a PR ostensibly limited to removing PathExclusionFilter. I'll post it once this one merges. It removes all of the fields for filters. And it creates the filters in a generic fashion in a loop. Gzip goes... moves to a BATS test. The handful of classes needing a filter (like debug or rate limiting) are given a method to look it up by type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18168: Remove path exclusion filter [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4243:
URL: https://github.com/apache/solr/pull/4243#issuecomment-4178798241
> Sadly the "Solr Script Tests" (iTest/bats) are flaky, at least when run via GHA. They passed locally. So I'm gonna merge this very soon.
>
> I'll throw up a separate PR to help us debug BATS tests in CI. I don't like the GHA script name "Solr Script Tests". We colloquially call these "BATS" so lets name it how we call it!
Love it!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org