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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
