Skip to content

Security considerations #16

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

Merged
merged 7 commits into from
Mar 30, 2023
Merged

Security considerations #16

merged 7 commits into from
Mar 30, 2023

Conversation

gkellogg
Copy link
Member

@gkellogg gkellogg commented Feb 17, 2023

Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Most are minor. SHOULD prevent is little better than MUST prevent, as that prevention remains an impossibility, and we should get away from specifying such an impossibility sooner than later.

Copy link
Contributor

@pfps pfps left a comment

Choose a reason for hiding this comment

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

This PR makes substantive changes that need discussion in the WG.

@domel
Copy link
Contributor

domel commented Feb 18, 2023

One does not exclude the other. I don't see why blocking this PR, there is a corresponding box with information in places for WG discussions.

@pfps
Copy link
Contributor

pfps commented Feb 18, 2023

I don't think the WG has even voted to take up this sort of change to RDF.

@gkellogg
Copy link
Member Author

This PR makes substantive changes that need discussion in the WG.

Can you be more specific? It includes issue markers, and extracts the security considerations from the mime type to its own section, which is called for in publishing standards. We have been working to correct issues in the security consideration section to address various issues. This text is used in a number of specs. Do you think the full WG needs to resolve to make changes to security considerations?

issue markers are not normative changes, but serve as cues to readers about issues under discussion that may lead to changes in future versions.

@pfps
Copy link
Contributor

pfps commented Feb 18, 2023

There are three separate changes in this PR. Security, IRIs, and JSON. I'm not aware that the WG has decided to consider any of them. I don't see any open issues at https://github.com/w3c/rdf-star-wg/issues where these changes could fit.

@gkellogg
Copy link
Member Author

There are three separate changes in this PR. Security, IRIs, and JSON. I'm not aware that the WG has decided to consider any of them. I don't see any open issues at https://github.com/w3c/rdf-star-wg/issues where these changes could fit.

Generally, these are related to errata, or the consequences of dealing with the errata.

  • Security Considerations is an issue for spec-track documents, as is Privacy Considerations. This is more a matter of moving text which involves making reasonable updates.
  • IRIs have been problematic due to the lack of progress on RFC3987 when prevents it from being normatively cited. We need to do something here, and issue Revise terminology to align with current RFCs. #15 is open to try to figure that out. In the mean time, an issue marker tells the community that it's being considered.
  • rdf:JSON is not an erratum, but it's been considered good practice to put issue markers in drafts about open issues.

It sounds like you think that the chairs need to set a policy on the scope of editorial changes related to dealing with errata and updated requirements for spec-track documents.

@TallTed
Copy link
Member

TallTed commented Feb 19, 2023

It sounds like you think that the chairs need to set a policy on the scope of editorial changes related to dealing with errata and updated requirements for spec-track documents.

Without speaking to the specifics of this PR (#16) or the discussion on it, I would suggest that keeping each PR (and each preceding Issue) to a single topic — by which I mean a single erratum when addressing errata from the previous spec — will be a better practice than mushing several topics/errata together, as is being done here.

@gkellogg
Copy link
Member Author

Generally, adding issue markers for open issues has been considered good practice, at least in groups I've been involved with. When doing a PR keeping the active issues listed in the spec was considered useful. However, here it seems to have become contentious.

I've removed the updated wording on the #15 and new issue marker for #14 to focus solely on the update to security considerations.

@gkellogg gkellogg changed the title Security considerations, IRIs, and JSON Security considerations Feb 19, 2023
@TallTed
Copy link
Member

TallTed commented Feb 20, 2023

Generally, adding issue markers for open issues has been considered good practice, at least in groups I've been involved with. When doing a PR keeping the active issues listed in the spec was considered useful. However, here it seems to have become contentious.

If this PR were just adding issue markers for open issues, I think there would have been no contention. However, it was adding those markers and making other changes to the content, which mix was unsurprisingly (to me) contentious...

gkellogg added a commit to w3c/rdf-n-quads that referenced this pull request Feb 20, 2023
@gkellogg gkellogg requested review from TallTed, afs and pfps February 20, 2023 07:08
@afs
Copy link
Contributor

afs commented Feb 20, 2023

The issue markers take the description from the issue.

But initial descriptions have been containing opinion and proposal and then do not reflect dicussion in the issue.

Here we have "Update issue on IRI to #15." Nothing to do with security considerations.

@TallTed
Copy link
Member

TallTed commented Feb 20, 2023

@afs wrote:

The issue markers take the description from the issue.

But initial descriptions have been containing opinion and proposal and then do not reflect dicussion in the issue.

Here we have "Update issue on IRI to #15." Nothing to do with security considerations.

Yes. This is another reason why (as I've noted elsewhere) I would prefer there to be one concern per Issue, and one Issue per PR, and to have meaningful naming and description of both Issue and PR. The initial Issue description need not be revised to reflect subsequent discussion, so long as it is a simple description of the concern with neither personal opinion nor proposed solution.

@gkellogg
Copy link
Member Author

@afs and @pfps, you both have outstanding changes requested that prevent merging this PR. Can you please either approve, or suggest additional changes?

This version should closely match that in w3c/rdf-n-quads#19 which has approvals.

@gkellogg gkellogg force-pushed the security-considerations branch from 9e6b2be to 40a2a74 Compare March 14, 2023 20:59
@gkellogg gkellogg added the spec:enhancement Change to enhance the spec without affecting conformance (class 2) –see also spec:editorial label Mar 16, 2023
Copy link
Contributor

@pfps pfps left a comment

Choose a reason for hiding this comment

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

The changes here now appear to be much better limited. The only problem is that the commit list has a lot of other stuff in it. It would be better to squash all the commits into one when merging or squash locally and force-push.

@gkellogg
Copy link
Member Author

My practice has been to do squash merges when there are more than a couple of commits.

A force push may become necessary if there are any conflicts with other PRs, but probably the time to quash is when ultimately merging the PR.

@gkellogg gkellogg merged commit 5ce4537 into main Mar 30, 2023
@gkellogg gkellogg deleted the security-considerations branch March 30, 2023 20:36
gkellogg added a commit to w3c/rdf-n-quads that referenced this pull request Mar 30, 2023
gkellogg added a commit to w3c/rdf-n-quads that referenced this pull request Mar 30, 2023
* Improve canonicalization section.
  * Reference issue #16 as a future direction for canonicalization.
* Add prohibition on using a datatype IRI if the datatype is xsd:string when canonicalizing.
* Update change note on PN_CHARS_U to describe the change in blank node representation.
* White space updates.
* Change note motivating the use of canonical N-Quads.
* Sync recent changes to w3c/rdf-concepts#16.
* Fix IRI term references.
* Update note motivating canonical N-Quads.

---------

Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Andy Seaborne <[email protected]>
Co-authored-by: Dan Yamamoto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:enhancement Change to enhance the spec without affecting conformance (class 2) –see also spec:editorial
Projects
None yet
5 participants