You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/11/16 16:35:12 UTC
[GitHub] [solr] janhoy opened a new pull request #418: Allow content-types with ;charset in ShowFileRequestHandler
janhoy opened a new pull request #418:
URL: https://github.com/apache/solr/pull/418
https://issues.apache.org/jira/browse/SOLR-XXXXX
--
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
[GitHub] [solr] janhoy commented on pull request #418: SOLR-15804 Allow content-types with ;charset in ShowFileRequestHandler
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #418:
URL: https://github.com/apache/solr/pull/418#issuecomment-971732727
I tested with 8.10, and put a css file in the config-set. It did not display well, and caused a JS stacktrace
But I think that is unrelated to this and has to do with the syntax highlighter we use. I'll file another JIRA, although CSS files here should not really happen...
--
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
[GitHub] [solr] janhoy commented on pull request #418: SOLR-15804 Allow content-types with ;charset in ShowFileRequestHandler
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #418:
URL: https://github.com/apache/solr/pull/418#issuecomment-972714512
I tested a bit more, and frontend uses `text/javascript` which is not a standard content-type, so it would get 400 error on those. I change files.js to use `application/javascript`, and also the xml types -> `application/xml` even if backend also supports `text/xml`.
--
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
[GitHub] [solr] janhoy commented on pull request #418: SOLR-15804 Allow content-types with ;charset in ShowFileRequestHandler
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #418:
URL: https://github.com/apache/solr/pull/418#issuecomment-972694399
> Given Solr has a proper ConfigSet API these days, I don't think we should worry about trying to support apps using ShowFile to download.
Absolutely.
I'll fix up the css highlighting bug (use txt instead), request `.html` with `text/xml`, test in main and then merge.
--
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
[GitHub] [solr] janhoy commented on pull request #418: SOLR-15804 Allow content-types with ;charset in ShowFileRequestHandler
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #418:
URL: https://github.com/apache/solr/pull/418#issuecomment-971734868
> Seems good... Only thought I had was that the adding of the mimetype here seemed odd, should it be added in the jetty config? Either `web.xml` or `webdefault.xml`? On the other hand, if this only matters in this handler, maybe that just confuses thinks more.
Guess Solr can fetch this data from ZK and Jetty would not know how to determine mime-type. So it is really local to this handler. I don't know any other valid uses of ShowFileRequestHandler except from serving the Files screen, so perhaps this is unnecessary functionality in the first place - perhaps it should always deliver content as plaintext? Or do other apps use it to "download" files from a configset?
--
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
[GitHub] [solr] janhoy commented on pull request #418: SOLR-15804 Allow content-types with ;charset in ShowFileRequestHandler
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #418:
URL: https://github.com/apache/solr/pull/418#issuecomment-971721914
To reveiewers:
So the whole idea of the original content-type screening was to avoid Solr presenting a file with an HTML content-type to the browser which would then render that HTML, which is clearly not the intention of the Files Admin UI either. So we silently fall-back `text/html` content-type to `text/plain` and only accept well-known content types.
The issue was that the Admin UI always attaches `;charset=utf-8` to the contentType param, and we did not accommodate for that, also I did not test the UI manually (stupid). This PR fixes this by checking the raw content-type, including lowercasing and we also accept `text/xml`.
It works well in 9.0 Admin UI for the files that I tested. The only thing that I see could still break is if a config-set contains some file that the Admin UI decides to request with an unknown content-type, then the 400 error would not be handled gracefully and result in the red error box on top with no message (that's another bug I think, that the error from the request is not rendered in the UI). But what I can see from [files.js](https://github.com/apache/solr/blob/main/solr/webapp/web/js/angular/controllers/files.js), it will not happen, as the UI only handles xml, html, js, json and css explicitly, and everything else requests `text/plain`. Perhaps the UI should map html to `text/plain` here since that is what will happen on the backend anyway.
--
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
[GitHub] [solr] janhoy merged pull request #418: SOLR-15804 Allow content-types with ;charset in ShowFileRequestHandler
Posted by GitBox <gi...@apache.org>.
janhoy merged pull request #418:
URL: https://github.com/apache/solr/pull/418
--
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
[GitHub] [solr] dsmiley commented on pull request #418: SOLR-15804 Allow content-types with ;charset in ShowFileRequestHandler
Posted by GitBox <gi...@apache.org>.
dsmiley commented on pull request #418:
URL: https://github.com/apache/solr/pull/418#issuecomment-972525429
Given Solr has a proper ConfigSet API these days, I don't think we should worry about trying to support apps using ShowFile to download.
--
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