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

Adding scheme to URL to support https in info.json #52

Merged
merged 4 commits into from
May 5, 2021

Conversation

glenrobson
Copy link
Contributor

This was raised in the cookbook meetings and is starting to cause mixed content errors with Mirador. The IIIF reference implementation is hosted over https but the info.json points to http. See the following example:

https://iiif.io/api/image/3.0/example/reference/918ecd18c2592080851777620de9bcb5-gottingen/info.json

This pull requests adds a scheme parameter to the config.

@coveralls
Copy link

coveralls commented Mar 6, 2021

Coverage Status

Coverage increased (+0.001%) to 98.008% when pulling 80a4daa on glenrobson:add_scheme_to_config into 57d5a80 on zimeon:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 98.009% when pulling fca1f2c on glenrobson:add_scheme_to_config into 57d5a80 on zimeon:master.

@glenrobson
Copy link
Contributor Author

Fixes: #36 (comment)

uri = "http://" + host
if (port != 80):
uri = scheme + "://" + host
if port != 80 and port != 443:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this this should really be:

if not ((port == 80 and scheme == 'http') or (port == 443 and scheme == 'https')):

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Simeon, I've made that change.

@zimeon zimeon merged commit 1019ecc into zimeon:master May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants