How to handle change with empty values for metadata?

[smatsson]

Hi!

I'm curious on how to handle this addition to the Creation extension (Metadata chapter):

"The value MAY be empty. In these cases, the space, which would normally separate the key and the value, MAY be left out."

It's not really that it is a hard part to understand but I'm having a bit of an issue for how this was added to the spec.

Looking at the PR (#90) there were some discussions in 2016 and then nothing before October last year when this change was merged without updating the version or date of the document (these still stand at 1.0.0 and 2016-03-25)

Updating the protocol without changing the version causes incompatibilities. By merging this PR you basically broke every 1.0.0 implementation out there since there is now no way of knowing what kind of metadata that can be provided.

A real life example:

tusdotnet validates and parses the Upload-Metadata header into a .NET class structure. Changing how this is done wouldcause a lot of problems for developers using tusdotnet.

Here's an example that worked perfectly before but will now cause weird bugs further down the pipe.
OnBeforeCreateAsync is a user specified function that is executed before a file is created. It is most often used to verify that all required metadata is provided.

OnBeforeCreateAsync = ctx =>
{

    if (!ctx.Metadata.ContainsKey("name"))
    {
        ctx.FailRequest("name metadata must be specified. ");
    }

    if (!ctx.Metadata.ContainsKey("contentType"))
    {
        ctx.FailRequest("contentType metadata must be specified. ");
    }

    return Task.CompletedTask;
}

ctx.Metadata.ContainsKey("name") will still succeed if the client provides a metadata key named "name" without any value. This will most likely cause other problems further down the road though as the value does not exist (but did in earlier specs of the protocol).

How should we handle these kind of changes? Should extensions be considered mutable? If so, how do we get notified on updates to the spec? It's a fine piece of documentation but I don't read it every week anyway =)

[Acconut]

without updating the version or date of the document (these still stand at 1.0.0 and 2016-03-25)

Thanks for pointing that out. I hoped to release a version 1.1 in the past but didn't get around to finish the organizational tasks around that yet.

Regarding your example: Are you concerned about metadata values being empty? Technically, the specification didn't explicitly disallow empty values in the past, as far as I always understood the specification. With the PR, the specification now explicitly allows empty values. Is that what you mean?

If so, how do we get notified on updates to the spec?

In the ideal world, we would announce it on the blog and so. But time has unfortunately been very limited for me recently. =/

[smatsson]

As you can see by my late reply, I don't have a lot of time either. Spare time is a very rare commodity these days. :)

Regarding your example: Are you concerned about metadata values being empty? Technically, the specification didn't explicitly disallow empty values in the past, as far as I always understood the specification. With the PR, the specification now explicitly allows empty values. Is that what you mean?

I see your point here. I guess one could have just base64 encoded an empty string and added that to the metadata header, which would indeed allow empty metadata and break my code in the first example. My concern is basically that I would change how metadata is parsed (from requiring key/value pairs to allowing key/value pairs or only a key) in tusdotnet and then break something for someone. For example if you look at the tus js client we have code that looks like this:

upload = new tus.Upload(file,
                {
                    endpoint: 'files/',
                    onError: onTusError,
                    onProgress: onTusProgress,
                    onSuccess: onTusSuccess,
                    metadata: {
                        name: file.name,
                        contentType: file.type
                    }
                });

In some browsers, depending on file type, the file.type property will return an empty string, causing an empty contentType metadata key. In earlier versions this would have caused a 400 Bad Request in tusdotnet but after the change it would be allowed. This might cause problems further down the line when the developer expects the data to be there but it's not.

I might be overthinking this whole thing though, it might not be an issue. As you said, one could just base64 encode an empty string and get the same behavior.

The current implementation is basically that I have added an enum to the code where the developer can chose how to parse metadata in case they need to revert to the old way for some reason. It's just to hard to anticipate all the ways someone could use the lib =)

In the ideal world, we would announce it on the blog and so. But time has unfortunately been very limited for me recently. =/

No worries mate. I think you are doing a great job with the protocol. I have set my watching settings in this repo to get notified of all conversations so hopefully I will be faster to implement new features next time.

[Acconut]

The current implementation is basically that I have added an enum to the code where the developer can chose how to parse metadata in case they need to revert to the old way for some reason. It's just to hard to anticipate all the ways someone could use the lib =)

That sounds like a good approach! As a library author I like to provide users with guarantees but I also want to avoid putting on too much chains, so a delicate split between innovation and compatibility is necessary, which is not always easy. But I think you made a good decision here 👍

No worries mate. I think you are doing a great job with the protocol. I have set my watching settings in this repo to get notified of all conversations so hopefully I will be faster to implement new features next time.

Thanks for the kind words! I hope to make the different protocol versions on the website more explicit soon.