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

Make canonicalizer be able to strip session id params even if they ar… #54

Merged
merged 1 commit into from
May 26, 2016

Conversation

vonrosen
Copy link
Contributor

…e the first params in the query string. And add session id strip test. And change IAURLCanonicalizer.java to ensure that if after transformations on the query string have completed and the query is empty, there is not a ? added to the end of the url.

…e the first params in the query string. And add session id strip test. And change IAURLCanonicalizer.java to ensure that if after transformations on the query string have completed and the query is empty, there is not a ? added to the end of the url.
@kris-sigur kris-sigur added this to the 1.1.7 Release milestone Mar 22, 2016
@johnerikhalse
Copy link
Contributor

johnerikhalse commented Apr 26, 2016

Will this require reindexing CDX'es?

If that's the case I will propose this goes into at least a minor release, or maybe a major release, and not into a bugfix release.

If IAURLCanonicalizer.java is not the default for CDX-indexer, OpenWayback and CDX-Server, then this change should be fine.

@kris-sigur
Copy link
Member

Yes, this should probably be deferred to 1.2 as it changes existing behavior.

Alternatively, if this is an IA only class, perhaps it should be deprecated?

@johnerikhalse
Copy link
Contributor

After reviewing OWB, it seems to me that the default configuration is not using IAURLCanonicalizer. The exception is CDX-Server, but since using CDX-Server is not the default at the moment, I think this PR is ok for the next bugfix release.

@kris-sigur kris-sigur merged commit 1485fdd into iipc:master May 26, 2016
@kris-sigur
Copy link
Member

Merged, mostly on the understanding that only IA uses this. Canonicalization really needs to be better standardized.

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.

3 participants