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

Upload fails in Safari if meta field value is an empty string #41

Open
arturi opened this issue Aug 7, 2016 · 3 comments
Open

Upload fails in Safari if meta field value is an empty string #41

arturi opened this issue Aug 7, 2016 · 3 comments

Comments

@arturi
Copy link
Contributor

arturi commented Aug 7, 2016

To reproduce:

var upload = new tus.Upload(file, {
  endpoint: '/upload',
  metadata: {
    foo: "hello",
    bar: "world",
    nonlatin: "słońce",
    empty: ""
  }
})

Safari says: An invalid or illegal string was specified when tus is setting the header here https://github.com/tus/tus-js-client/blob/master/lib/upload.js#L222-L228.

Possible solutions: remove key if value is empty? But what if it’s intentional? Maybe set it to a string that’s empty ('""') like:

for (var key in metadata) {
  var value = metadata[key];
  if (value === "") {
    value = "''";
  }
  encoded.push(key + " " + Base64.encode(metadata[key]));
}
@Acconut
Copy link
Member

Acconut commented Aug 24, 2016

Thank you for pointing this out.

Possible solutions: remove key if value is empty? But what if it’s intentional? Maybe set it to a string that’s empty ('""') like:

That's a very bad idea, since this is not compatible with the specification:

The Upload-Metadata request and response header MUST consist of one or more comma-separated key-value pairs. The key and value MUST be separated by a space. The key MUST NOT contain spaces and commas and MUST NOT be empty. The key SHOULD be ASCII encoded and the value MUST be Base64 encoded. All keys MUST be unique.

Basically, if you have an empty value for a specific key, the Base64-encoded digest will also be empty, such as:

Upload-Metadata: emptyValue ,someOtherStuff b4s564Enc==

I will add a proper test and fix for this.

@Acconut
Copy link
Member

Acconut commented Aug 24, 2016

The actual problem with this sitation does not sit inside the Base64-encoding step but rather when we try to set the Upload-Metadata header. Safari (and no other browser, to our knowledge) does not allow header values to end with a space:

> xhr = new XMLHttpRequest()
> xhr.open("GET", "")
> xhr.setRequestHeader("foo", "bar ")
< Error: SyntaxError: DOM Exception 12

In order to fix this, we probably need to change the specification to allow removing this last space.

@Acconut
Copy link
Member

Acconut commented Aug 28, 2016

The change has been proposed in tus/tus-resumable-upload-protocol#90.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants