ad-freiburg / qlever

Very fast SPARQL Engine, which can handle very large knowledge graphs like the complete Wikidata, offers context-sensitive autocompletion for SPARQL queries, and allows combination with text search. It's faster than engines like Blazegraph or Virtuoso, especially for queries involving large result sets.
Apache License 2.0
417 stars 52 forks source link

Fix the content-type check of SERVICE responses #1490

Closed UNEXENU closed 2 months ago

UNEXENU commented 2 months ago

Fixes #1489. With this fix, QLever again accepts responses of a SERVICE request where the content-type has additional fields, e.g. application/sparql-results+json;charset=utf-8.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.15%. Comparing base (2e47358) to head (fdb8768). Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1490 +/- ## ========================================== + Coverage 94.13% 94.15% +0.01% ========================================== Files 347 347 Lines 25620 25621 +1 Branches 3444 3444 ========================================== + Hits 24118 24123 +5 + Misses 1460 1456 -4 Partials 42 42 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ahankinson commented 2 months ago

Thanks!

I note that MDN says that Media Types are case-insensitive, but are generally written in lower-case. Should there be a normalization of that as well?

https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types#structure_of_a_mime_type

joka921 commented 2 months ago

@ahankinson That is probably a good idea with the lowercasing. @UNEXENU Can you add a lowercased test (we have a utf-aware to lower function somewhere in StringUtils.h). You can then also a unit test for a cased content-type with an additional charset parameter, s.t. this regression would have been found in the first place.

ahankinson commented 2 months ago

This looks good to me!

ahankinson commented 2 months ago

@UNEXENU are those commits supposed to be on this branch?

UNEXENU commented 2 months ago

@ahankinson While not directly related to the issue, they're intended to be fixed on this branch.

sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud