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

fix: azure blob file not found #11674

Closed
wants to merge 4 commits into from
Closed

fix: azure blob file not found #11674

wants to merge 4 commits into from

Conversation

IsQiao
Copy link

@IsQiao IsQiao commented May 10, 2022

Fixes #11673

@Skrypt
Copy link
Contributor

Skrypt commented May 10, 2022

The fix is questionnable. Needs investigation.

@Skrypt Skrypt requested a review from deanmarcussen May 10, 2022 15:34
@@ -145,7 +145,7 @@ await foreach (var blob in page)
}
else
{
var itemName = Path.GetFileName(WebUtility.UrlDecode(blob.Blob.Name)).Trim('/');
var itemName = Path.GetFileName(blob.Blob.Name).Trim('/');
Copy link
Member

Choose a reason for hiding this comment

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

The decode code is old from the previous blob library from Microsoft.

It maybe that it is no longer delivered by the azure library encoded, so a decode doesn't do anything useful.

Copy link
Author

@IsQiao IsQiao May 11, 2022

Choose a reason for hiding this comment

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

the decode can damage file path. aaa+++.jpg -> aaa .jpg
remove decode can resolve it if use Kestrel Server.
but need to set allowDoubleEscaping="true" in web.config if use IIS Server.

Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

Copy link
Member

@Piedone Piedone left a comment

Choose a reason for hiding this comment

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

I sincerely apologize for us taking so much time here. I checked out the PR, going over old ones, and this would be quite useful.

Is this something you'd like to revisit any time soon or should we close?

Copy link
Member

Choose a reason for hiding this comment

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

Please check the decode in GetDirectoryContentFlatAsync() too.

While this indeed fixes the linked bug, I'm not sure if it'll break anything else. Can you upload a file to Blob Storage with a name that has all kinds of characters that should be encoded for a URL, and it'll keep them in their original form?

Copy link
Author

Choose a reason for hiding this comment

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

ok, i will to test all kinds of chars for this case and resolve the PR conflicts. please wait.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you!

@Piedone
Copy link
Member

Piedone commented Apr 21, 2024

Not to be a bother, but any news here, @IsQiao?

@IsQiao
Copy link
Author

IsQiao commented Apr 22, 2024

Sorry, I found new bugs regarding this fix and I will fix them.

@Piedone
Copy link
Member

Piedone commented Apr 22, 2024

Great, thanks! When you're done addressing all feedback of a review, click "Re-request review" in the top-right corner for each reviewer when you're ready for another round of review, so they know that you're done.

@IsQiao
Copy link
Author

IsQiao commented Apr 22, 2024

Great, thanks! When you're done addressing all feedback of a review, click "Re-request review" in the top-right corner for each reviewer when you're ready for another round of review, so they know that you're done.

ok, i will click the button after fix the bug. thanks

@Piedone
Copy link
Member

Piedone commented May 27, 2024

This needs to be fixed not just for Blob Storage, see #11673 (comment).

Copy link
Contributor

It seems that this pull request didn't really move for quite a while. Is this something you'd like to revisit any time soon or should we close? Please comment if you'd like to pick it up.

@github-actions github-actions bot added the stale label Jul 27, 2024
Copy link
Contributor

Closing this pull request because it has been stale for very long. If you think this is still relevant, feel free to reopen it.

@github-actions github-actions bot closed this Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plus signs in Media file or folder name break Media Library links and previews
4 participants