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

Return 404 for versioned I-D xml2rfc-style paths #157

Closed
strogonoff opened this issue Mar 27, 2022 · 15 comments
Closed

Return 404 for versioned I-D xml2rfc-style paths #157

strogonoff opened this issue Mar 27, 2022 · 15 comments
Assignees
Labels
API enhancement New feature or request xml2rfc Related to APIs for compatibility with xml2rfc consumers

Comments

@strogonoff
Copy link
Collaborator

Based on https://github.com/ietf-ribose/relaton-data-ids/issues/6#issuecomment-1047774208

@strogonoff strogonoff added enhancement New feature or request xml2rfc Related to APIs for compatibility with xml2rfc consumers API labels Mar 27, 2022
@strogonoff strogonoff self-assigned this Mar 27, 2022
@ronaldtse
Copy link
Collaborator

Specification:

Correct and should continue working:

  • Unversioned I-D has path pattern: reference.I-D.xxx.xml
  • Versioned I-D has path pattern: reference.I-D.draft-xxx-nn.xml

In the following cases, the path should return 404:

  • Unversioned I-D has path pattern: reference.I-D.draft-xxx.xml
  • Versioned I-D has path pattern: reference.I-D.xxx-nn.xml

This change ONLY applies to I-Ds.

@strogonoff
Copy link
Collaborator Author

Yes, that’s the understanding

@strogonoff
Copy link
Collaborator Author

strogonoff commented Mar 29, 2022

For the sake of posterity, the existing logic was based on this requirement:

Pattern 2: http://xml2rfc.ietf.org/public/rfc/bibxml-ids/reference.I-D.draft-example-name-99.xml

and was intentionally designed to handle any combination of identifier with/without draft- prefix and with/without trailing version:

https://github.com/ietf-ribose/bibxml-service/blob/a99c2d7db091e17cbdc7903f80ce417bb14c57dd/xml2rfc_compat/fetchers.py#L58-L123

@strogonoff
Copy link
Collaborator Author

strogonoff commented Mar 29, 2022

New behavior (in effect after next version is deployed) described below.

The path is used as a general example of any I-D path.

No change:

  • /public/rfc/bibxml-ids/reference.I-D.weis-gdoi-iec62351-9.xml returns Relaton-sourced XML
  • /public/rfc/bibxml-ids/reference.draft-weis-gdoi-iec62351-9.xml returns Relaton-sourced XML
  • /public/rfc/bibxml-ids/reference.I-D.draft-weis-gdoi-iec62351-9.xml returns Relaton-sourced XML

Changed:

  • /public/rfc/bibxml-ids/reference.I-D.weis-gdoi-iec62351-9-00.xml returns

    {"error": {"message": "Error resolving bibliographic item. Tried methods: auto (internet_drafts): not found (Versioned I-D references are not supported), fallback (): not indexed"}}

Conflict:

To make the last instance return 404, ideally bibxml-data-archive repository should not contain a versioned reference (the basic requirement is “any preexisting path must resolve to maintain backwards consistency”, and bibxml-data-archive is used as a snapshot of preexisting paths).

If the above is OK, this can be closed.

@ronaldtse
Copy link
Collaborator

ronaldtse commented Mar 29, 2022

New behavior (in effect after next version is deployed) described below.

No change:

  • /public/rfc/bibxml-ids/reference.I-D.weis-gdoi-iec62351-9.xml returns Relaton-sourced XML

This is correct.

  • /public/rfc/bibxml-ids/reference.draft-weis-gdoi-iec62351-9.xml returns Relaton-sourced XML

/public/rfc/bibxml-ids/reference.draft-weis-gdoi-iec62351-9.xml leads to an unversioned I-D, and should return 404, as per "Unversioned I-D has path pattern: reference.I-D.xxx.xml"

  • /public/rfc/bibxml-ids/reference.I-D.draft-weis-gdoi-iec62351-9.xml returns Relaton-sourced XML

/public/rfc/bibxml-ids/reference.I-D.draft-weis-gdoi-iec62351-9.xml leads to an unversioned I-D, and should return 404, as per "Unversioned I-D has path pattern: reference.I-D.xxx.xml"

Changed:

  • /public/rfc/bibxml-ids/reference.I-D.weis-gdoi-iec62351-9-00.xml returns

    {"error": {"message": "Error resolving bibliographic item. Tried methods: auto (internet_drafts): not found (Versioned I-D references are not supported), fallback (): not indexed"}}

Correct.

Conflict:

This should not be marked as a conflict, and should be marked as correct.

As described earlier, the versioned I-D has path pattern reference.I-D.draft-xxx-nn.xml.

@ronaldtse
Copy link
Collaborator

For the sake of posterity, the existing logic was based on this requirement:

Pattern 2: http://xml2rfc.ietf.org/public/rfc/bibxml-ids/reference.I-D.draft-example-name-99.xml

and was intentionally designed to handle any combination of identifier with/without draft- prefix and with/without trailing version:

Fully understand. It's just that @TonyLHansen has clarified, after the resolver was completed, that the implementation should now only accept the two described patterns and treat the others as unacceptable. So we have to adjust the resolving mechanism. Thanks!

@strogonoff
Copy link
Collaborator Author

@ronaldtse I see, apparently I have failed to read the spec carefully. Updated behavior is this:

  • /public/rfc/bibxml-ids/reference.I-D.weis-gdoi-iec62351-9.xml returns Relaton-sourced XML

  • /public/rfc/bibxml-ids/reference.I-D.weis-gdoi-iec62351-9-00.xml returns this:

    {"error": {"message": "Error resolving bibliographic item. Tried methods: auto (internet_drafts): not found (unsupported xml2rfc-style I-D reference: possibly missing I-D prefix or unexpected draft- prefix and trailing version combination), fallback (): not indexed"}}

  • /public/rfc/bibxml-ids/reference.draft-weis-gdoi-iec62351-9.xml returns the same error message as above.

  • /public/rfc/bibxml-ids/reference.I-D.draft-weis-gdoi-iec62351-9-00.xml returns Relaton-sourced XML.

@strogonoff
Copy link
Collaborator Author

There is an additional fix to be implemented, let me update this issue before closing.

@strogonoff
Copy link
Collaborator Author

strogonoff commented Mar 29, 2022

Nope, all good.

Logic for validating given path: https://github.com/ietf-ribose/bibxml-service/blob/5a069257321652b8acd810e8ff28a89d1a4829d1/xml2rfc_compat/fetchers.py#L85-L94

ref is full filename except for reference. and XML extension; bare_ref has prefixes stripped but still contains a trailing version if given.

If given ref has the version, PostgreSQL JSON lookup will only match the document with given version, otherwise it will retrieve all documents reverse-sorted by draft version number of which we’ll use the first one (largest version number):

https://github.com/ietf-ribose/bibxml-service/blob/5a069257321652b8acd810e8ff28a89d1a4829d1/xml2rfc_compat/fetchers.py#L102-L118

@strogonoff
Copy link
Collaborator Author

Looks like it’s working as expected now, closing.

@ronaldtse
Copy link
Collaborator

Thanks @strogonoff !

@TonyLHansen
Copy link

TonyLHansen commented Mar 29, 2022

I have a problem with one of the examples above that doesn't match the agreed-to patterns:

/public/rfc/bibxml-ids/reference.draft-weis-gdoi-iec62351-9.xml

This doesn't have the "I-D" in it. It also has "draft-" without the "-nn" at the end.

Is this example being accepted erroneously? I may have just gotten confused by all the back and forth.

@ronaldtse
Copy link
Collaborator

@TonyLHansen sorry for the confusion in general with this issue.

I believe as @strogonoff stated:

/public/rfc/bibxml-ids/reference.draft-weis-gdoi-iec62351-9.xml returns the same error message as above.

This path is not accepted.

@TonyLHansen
Copy link

Thanks for confirming, @ronaldtse
Thumbs up!

@ronaldtse
Copy link
Collaborator

Thank you @TonyLHansen for the patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API enhancement New feature or request xml2rfc Related to APIs for compatibility with xml2rfc consumers
Projects
None yet
Development

No branches or pull requests

3 participants