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

add encoded filename as part of delete api url #3141

Merged
merged 5 commits into from
Dec 6, 2023

Conversation

prakashsvmx
Copy link
Member

@prakashsvmx prakashsvmx commented Dec 1, 2023

@cesnietor , @poornas

Console has different API (endpoint) urls for different scenarios to handle deletes.

the below url is modified to pass filename in case of single object delete, in url params

http://localhost:5005/api/v1/buckets/test-v-bucket/objects?prefix=MS50eHQ%3D&non_current_versions=true&bypass=false

Context, this is done so that we can do special operations with apis that have the prefix defined. single objects where being sent in the array so a single delete object api was needed.

@prakashsvmx prakashsvmx self-assigned this Dec 1, 2023
@cesnietor
Copy link
Collaborator

cesnietor commented Dec 1, 2023

I see what you are saying @prakashsvmx .
Humm here let's consider a multiple object call only those that delete different object names, i.e. if it's a single file with multiple versions to be deleted (due to a versioned bucket), that for us is still a single delete call, with the all-versions enabled.

so here we don't need to do any changes to the /delete-objects call but rather have a separate one for single-object delete. so that we can plug the name and the path in the prefix param. In the UI if multiple files are selected then we use the /delete-objects, if it's a single file we just use the new one.

@prakashsvmx prakashsvmx changed the title add encoded filename as part of delete url for tooling [WIP] add encoded filename as part of delete url for tooling Dec 5, 2023
@prakashsvmx
Copy link
Member Author

prakashsvmx commented Dec 6, 2023

@cesnietor
the single delete is already implemented as part of http://localhost:5005/api/v1/buckets/{bucket_name}/objects API endpoint with DELETE method.

so i have renamed path to prefix to be consistent everywhere. and updated all single deletes to use this endpoint.

@poornas @kannappanr

@prakashsvmx prakashsvmx changed the title [WIP] add encoded filename as part of delete url for tooling add encoded filename as part of delete url for tooling Dec 6, 2023
@prakashsvmx prakashsvmx changed the title add encoded filename as part of delete url for tooling add encoded filename as part of delete api url Dec 6, 2023
@cesnietor
Copy link
Collaborator

@cesnietor the single delete is already implemented as part of http://localhost:5005/api/v1/buckets/{bucket_name}/objects API endpoint with DELETE method.

so i have renamed path to prefix to be consistent everywhere. and updated all single deletes to use this endpoint.

@poornas @kannappanr

Ahh awesome, thanks.

@allanrogerr
Copy link
Contributor

@prakashsvmx Please check
Vulnerability #1: GO-2023-2382
Vulnerability #2: GO-2023-21

@cesnietor
Copy link
Collaborator

@allanrogerr vulns are unrelated to this change. They've been there for a while. We'll have to address them separately.

@allanrogerr
Copy link
Contributor

@allanrogerr vulns are unrelated to this change. They've been there for a while. We'll have to address them separately.

Thats unfortunate. Vulnerabilities should take some priority IMO.

@allanrogerr
Copy link
Contributor

@prakashsvmx Will encoding the filename of multiple objects be addressed in a followup PR? That is encoding the files name of the payload of api/v1/buckets/<bucket-name>/delete-objects ?

Copy link
Contributor

@allanrogerr allanrogerr left a comment

Choose a reason for hiding this comment

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

Tested. Please consider fixing vulnerabilities.

@bexsoft
Copy link
Collaborator

bexsoft commented Dec 6, 2023

Tested. Please consider fixing vulnerabilities.

Fixed in #3146

@cesnietor
Copy link
Collaborator

cesnietor commented Dec 6, 2023

@prakashsvmx Will encoding the filename of multiple objects be addressed in a followup PR? That is encoding the files name of the payload of api/v1/buckets//delete-objects ?

for multiple files we send them in the payload, so no need to encode them @allanrogerr

Copy link
Collaborator

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

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

tested LGTM

@cesnietor cesnietor merged commit 6767bfa into minio:master Dec 6, 2023
cesnietor pushed a commit to cesnietor/console that referenced this pull request Jan 12, 2024
cesnietor pushed a commit to cesnietor/console that referenced this pull request Jan 12, 2024
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

Successfully merging this pull request may close these issues.

4 participants