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

Ensure we find the filename in Upload-Metadata when clients send extra #82

Conversation

drastik
Copy link

@drastik drastik commented Sep 29, 2018

I wrote a Drupal integration (https://www.drupal.org/project/tus), and we need a few extra values passed from Uppy in the meta{} option.
entityType, entityBundle, fieldName (for determining folder storage based on user's settings within the CMS).

tus-php assumes a single value, or that the first value in the Upload-Metadata header is filename.
When I plug in the extra values in Uppy, filename occurs last, and tus-php fails to find filename properly.

This will fix that.

@drastik
Copy link
Author

drastik commented Oct 1, 2018

@ankitpokhrel Is this good to go?

Let me know if you want any code style changes, this is a very important patch though.

@ankitpokhrel
Copy link
Owner

@drastik tests are failing and also I am not ok with the changes.

For me there should be a separate method say extractMeta or similar that will take the key and return value for that key. It should follow rules for upload meta mentioned in tus protocol. extractFilename can then use extractMeta method to return file name.

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.

}

return base64_decode($file);
return $metaValues['filename'];
Copy link
Owner

Choose a reason for hiding this comment

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

uppy uses name as key instead of filename so we can't just use filename directly

@samundra
Copy link
Collaborator

samundra commented Oct 2, 2018

I will address the changes that @ankitpokhrel has asked for. I have a hunch what needs to be done and will fix the breaking tests as well. Besides, I will try to have a look into drupal module as well and provide feedback by tonight. Happy Hacktoberfest guys.

@ankitpokhrel
Copy link
Owner

Fixed in #84

@ankitpokhrel ankitpokhrel added this to the 0.1.0 milestone Oct 20, 2018
@ankitpokhrel ankitpokhrel added this to Done in Kanban Oct 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Kanban
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants