Skip to content

fix: Add IANA type to child metadata in FileDataAccessor - #1237

Merged
joachimvh merged 2 commits into
versions/4.0.0from
fix/iana
Mar 28, 2022
Merged

joachimvh merged 2 commits into
versions/4.0.0from
fix/iana

Conversation

@joachimvh

Copy link
Copy Markdown
Member

📁 Related issues

Closes #1207

✍️ Description

No breaking changes so can go directly into main.

@joachimvh joachimvh added the semver.patch Does not require a minor or major version bump label Mar 22, 2022
@joachimvh
joachimvh requested a review from RubenVerborgh March 22, 2022 10:32
this.addPosixMetadata(metadata, childStats);
// Containers will not have a content-type
if (childLink.contentType) {
metadata.add(RDF.terms.type, toNamedTerm(`${IANA.namespace}${childLink.contentType}#Resource`));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We'll need to sanity-check childLink.contentType unfortunately. Unless this is done elsewhere—but still. We don't want to be tricked into writing weird things.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What exactly is the potential issue that you see here? It will always just be a named node in a triple, so converters should have no issue with it to convert it to valid turtle (or other format).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

childLink.contentType = 'a very weird content type' resulting in an invalid RDF term, hence invalid RDF, hence parser failure.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If we write invalid RDF I would see that as an issue with our serializer since that is going to convert this NamedNode to turtle and is responsible for generating valid RDF. But I'll just add a parseContentType wrapped by a try/catch here to make sure the request doesn't fail if there is a child with an invalid type.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nah, RDF/JS assumes that created named nodes are valid. We could have a sanity check in toNamedTerm, but I think it's better here.

an issue with our serializer

This would mean that the serializer checks every single term; would be a massive slowdown. Rather best to assume that NamedNode behaves correctly.

@joachimvh
joachimvh changed the base branch from main to versions/4.0.0 March 28, 2022 08:44
@joachimvh
joachimvh merged commit 027e370 into versions/4.0.0 Mar 28, 2022
@joachimvh
joachimvh deleted the fix/iana branch March 28, 2022 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver.patch Does not require a minor or major version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose all required fields in container listings for the filesystem backend

2 participants