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

OGC service params should be case insensitive #366

Closed
nickdos opened this issue Feb 14, 2019 · 8 comments · Fixed by #458
Closed

OGC service params should be case insensitive #366

nickdos opened this issue Feb 14, 2019 · 8 comments · Fixed by #458

Comments

@nickdos
Copy link
Contributor

nickdos commented Feb 14, 2019

Currently some OGC services (e.g. WMS tiles) require the OGC params to be provided in upper case. The problem is that recent versions of Leaflet send the various dynamic params in lower case by default and the tiles fail to load. There is an option in Leaflet to send params in upper case (uppercase: true) but this sends all params in upper case and then the ALA-defined params are ignored. E.g. LAT and LON are ignored, as they are required to sent in lowercase!

The OGC spec says params should not be case sensitive:

http://cite.opengeospatial.org/OGCTestData/wms/1.1.1/spec/wms1.1.1.html#basic_elements.param_rules.order_and_case

@nickdos
Copy link
Contributor Author

nickdos commented Feb 14, 2019

@ansell suggested:

This option using a Filter looks like it could possibly be supportable by biocache-service without changing the way RequestParam works https://stackoverflow.com/questions/29531786/making-a-request-parameter-binding-case-insensitive/29533456

@adam-collins
Copy link
Contributor

Fix for this is on docvalues branch.

@djtfmartin
Copy link
Member

@adam-collins ive reverted the commits for case-insensitivity as they broke WMS

@KeyboardSounds
Copy link
Contributor

@adam-collins @djtfmartin this issue is affecting us (see here), and I'd like to take a crack at fixing it. If I put together a PR, would it be possible for you to review, (and if you're happy with it) merge and release?

@djtfmartin
Copy link
Member

Thanks @KeyboardSounds ! Yes that would be great.

@Rita-C
Copy link
Contributor

Rita-C commented Jun 19, 2020

Tested on https://biocache-test.ala.org.au/
Issue ticket and PR can be moved to "CAB ready" once code reviewed, currently assigned to @brucehyslop

@nickdos
Copy link
Contributor Author

nickdos commented Jul 16, 2020

Re-opening until deployed to prod (done column)

@nickdos nickdos reopened this Jul 16, 2020
@djtfmartin
Copy link
Member

Closing historic issue. If this is still a problem, please reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants