Latest Version of htmlized paged fails if name of doc contains -1

[WhippsP]

Describe the issue

A very specific use case where if a document name contains -1 at the end it may conflict with another document with a revision of -01

Current Example: This link should get the the latest version (13 currently)
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1
However it errors.

Code of Conduct

• I agree to follow the IETF's Code of Conduct

[aaronpk]

Coming up on 4 years of this bug: #3417

[WhippsP]

Actual Issue stems from here

Need to either change the regex at this line or modify settings.URL_REGEXPS for rev so it does not look for 1 or 2 numbers but just 2.

The current regex brings the revision in as 1-01 and the name as draft-ietf-oauth-v2 which is why we are getting 404’s.

There are currently 3 places that use "rev" from URL_REGEXPS

• ietf/doc/urls.py
• ietf/doc/urls_statement.py
• ietf/api/urls.py

As it goes against the naming standard for documents for revision to only have 1 number I have submitted a PR #9534 to update the Rev regex to only allow 2 numbers

[rjsparks]

This problem is more complicated than you've discovered so far.

The proposed fix at #9534 will cause URLs in proceedings, such as https://datatracker.ietf.org/doc/slides-93-ccamp-5/ to start to fail (with a NoReverseMatch). There are other, similar, issues with other document types.

Perhaps the faster way to resolve the specific problem at the /html/ endpoint is to focus specifically there (in the view, adjusting the URLs to get to the view if necessary) on its handling for documents of type draft.

[WhippsP]

Thanks for the feedback @rjsparks, maybe I am missing something but I can not see any url match that uses REV that would apply to your specific example? That URL would still be applied to document_main with no rev passed the same as it does now.

I have created a PR ( #9538 ) for a second option which would be to update the htmlized regex directly. As you mention this would be the faster way to resolve but not sure its the cleanest.

[rjsparks]

maybe I am missing something but I can not see any url match that uses REV that would apply to your specific example?

It's in the code that constructs the URL in reverse (given the name and the view) in the template for the revision list:

datatracker/ietf/templates/doc/revisions_list.html

Line 34 in c4d69d0

href="{% if document_html %}{% url 'ietf.doc.views_doc.document_html' name=doc.name rev=rev %}{% else %}{% url 'ietf.doc.views_doc.document_main' name=doc.name rev=rev %}{% endif %}"

If you access /doc/slides-93-ccamp-5/ in a dev environment running your original PRs branch, you'll get the full stacktrace of the failure.

Your second PR avoids this particular pitfall. I'll look to see if it's lucky enough to avoid the others.

But as I noted above, this is much more complicated than it appears, or it would have been fixed long ago. The document_main view is a kitchen sink that's been patched into for decades and the document_html view relies on its idiosyncrasies. It is slated for re-implementation when we have the infrastructure changes finished that will prerender things like the htmlized views and store/serve them as blobs.

[WhippsP]

Ahh I see, so the naming standard for versions is not consistent across documents. So /doc/slides-93-ccamp-5/1/ would actually pass a name of "5" and an empty Rev with the the option 1 update to the regex in settings. Maybe if the standards are different for draft there should be a draftrev?

URL_REGEXPS = { ... "name": r"(?P<name>[A-Za-z0-9._+-]+?)", "rev": r"(?P<rev>[0-9]{1,2}(-[0-9]{2})?)", "draftrev": r"(?P<rev>[0-9]{2}(-[0-9]{2})?)", }

or if this is slated to be updated then the quicker direct update to html_document related regex as per the PR might be better.

[rjsparks]

Yes, the patch to the /doc/html processing is less likely to have unintended consequences given the current code.
I'm moving further commentary on that patch to #9538, which looks like it will help.

[rjsparks]

An aside:

Ahh I see, so the naming standard for versions is not consistent across documents.

Well, there is, but it couldn't be retroactively applied against some number of very old non-draft documents that existed before the draft name conventions became standardized. Working over the pile of artifacts this organization has created over the decades requires adapting to occasional changes of convention, some of them abrupt and jarring, but many subtle and more a product of slow drift of culture than any given explicit decision.

And there's pain from past errors that the tooling introduced - we have a big set of review artifacts that are named badly because of a long running bug that will either have to be dealt with by renaming, or we carry an exception around in the handling code for them forever.

[rjsparks]

Fixed by #9538