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

p:inline needs a fix as consequence of the RFC-valid URIs only decision #916

Closed
xml-project opened this issue Nov 21, 2019 · 8 comments
Assignees

Comments

@xml-project
Copy link
Contributor

@xml-project xml-project commented Nov 21, 2019

Currently we say:

The base URI of the document is the base URI of the p:inline element or of the parent element in the case of an implicit inline.

  • There needs to be said that it is an error, if the documents base URI is not RFC valid.
  • We need to say whether the processor has to guarantee that the base URI of the p:inline (or the parent element in case of an implicit inline) is RFC valid or if the pipeline author has to take care of this by using "xml:base".
@gimsieke

This comment has been minimized.

Copy link
Contributor

@gimsieke gimsieke commented Nov 21, 2019

We probably need to say that the static base URI and any @xml:base (after being made absolute if need be) must be RFC-valid. The processor needs to take care of the static base URI, the user of @xml:base.

@ndw

This comment has been minimized.

Copy link
Contributor

@ndw ndw commented Nov 21, 2019

This decision is having a much bigger blast radius than I imagined it was going to have. People are sloppy with URIs and most of the time it works out ok. Being extraordinarily pedantic about them isn't going to win us accolades from any users.

@gimsieke

This comment has been minimized.

Copy link
Contributor

@gimsieke gimsieke commented Nov 21, 2019

This pedantry is somehow alleviated by the fact that we say to users: You can always sanitize most sloppy file paths / URIs by applying p:urify() to them. We can specify that p:urify() also tries to parse non-file: URIs and percent-encode components like path segments also in HTTP URIs etc.
On the other hand, I have been comfortable with accepting any string and see which underlying error may happen: URI is not hierarchical, XOM complains, …
But then we said in the interest of interoperability we should require a) compliance and b) clearly defined normalization operations applied to the strings with the goal of turning most of the sloppiness out there into RFC 3986 gold (at the user’s request).

@ndw

This comment has been minimized.

Copy link
Contributor

@ndw ndw commented Nov 21, 2019

Per 21 Nov editor's call, we're going to be strict in the spec and in the test suite. Implementors may provide switches to be more relaxed. We accept that users may push back on this decision, but we're going to stand by our decision.

@xatapult

This comment has been minimized.

Copy link
Contributor

@xatapult xatapult commented Nov 21, 2019

Together with #889

@gimsieke

This comment has been minimized.

Copy link
Contributor

@gimsieke gimsieke commented Nov 27, 2019

Is addressed by PR #918. There is now a new sentence for p:inline (italicized here):

The base URI of the document is the base URI of the p:inline element or of the parent element in the case of an implicit inline. If document-properties provides a value for “base-uri”, this value is the base URI of the document. It is a dynamic error (err:XD0064) if the base URI is not both absolute and valid according to [RFC 3986].

It has a slightly different wording than the other occurrence of D0064 in p:document. Should we also add “both absolute and valid” instead of just “valid” to this other occurrence, or should we assign a new error number to one of the occurrences?

@xml-project

This comment has been minimized.

Copy link
Contributor Author

@xml-project xml-project commented Nov 27, 2019

I thing in all three cases: (1) p:inline, (2) p:document/p:load and (3) p:set-document-properties the base URI must be valid and absolute. I therefor agree to use the proposed text and change the text at the other positions.

This was referenced Nov 29, 2019
@xml-project

This comment has been minimized.

Copy link
Contributor Author

@xml-project xml-project commented Nov 29, 2019

fixed with #277 and #919

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.