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: Use existing name to construct new file path based on extension #520

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Feb 15, 2024

Should fix #236

Don't have a way to test right now as my office online server refuses to open doc files at all without meaningful error messages, but this should prevent an issue i've seen described where the file was stored as "New File.docx" and could then not be reopened when converting from doc to docx.

@juliusknorr juliusknorr added 3. to review Waiting for reviews bug Something isn't working labels Feb 15, 2024
Base automatically changed from feat/28 to main February 15, 2024 10:30
@@ -562,7 +562,8 @@ public function putFile($fileId,
$suggested = iconv('utf-7', 'utf-8', $suggested);

if ($suggested[0] === '.') {
$path = dirname($file->getPath()) . '/New File' . $suggested;
$rawName = pathinfo($file->getName(), PATHINFO_FILENAME);
Copy link
Member

Choose a reason for hiding this comment

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

If i understand correctly, X-WOPI-SuggestedTarget provides either a file name or a extension. Now in this case, when the provided string starts with a '.', pathinfo will always provide an empty string for PATHINFO_FILENAME, according to the documentation.

Unless a path is provided with this header, which is not what I understand it transports, but i would not exclude it entirely… but then a check would be missing.

https://3v4l.org/#live

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants