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 invalid path for windows dialogs #4019

Merged
merged 2 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,24 @@ package cfd

import "fmt"

var errUnsupported = fmt.Errorf("common file dialogs are only available on windows")
var unsupportedError = fmt.Errorf("common file dialogs are only available on windows")

// TODO doc
func NewOpenFileDialog(config DialogConfig) (OpenFileDialog, error) {
return nil, errUnsupported
return nil, unsupportedError
}

// TODO doc
func NewOpenMultipleFilesDialog(config DialogConfig) (OpenMultipleFilesDialog, error) {
return nil, errUnsupported
return nil, unsupportedError
}

// TODO doc
func NewSelectFolderDialog(config DialogConfig) (SelectFolderDialog, error) {
return nil, errUnsupported
return nil, unsupportedError
}

// TODO doc
func NewSaveFileDialog(config DialogConfig) (SaveFileDialog, error) {
return nil, errUnsupported
return nil, unsupportedError
}
17 changes: 16 additions & 1 deletion v2/internal/go-common-file-dialog/cfd/DialogConfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

package cfd

import "fmt"
import (
"fmt"
"os"
"reflect"
)

type FileFilter struct {
// The display name of the filter (That is shown to the user)
Expand All @@ -11,6 +15,9 @@ type FileFilter struct {
Pattern string
}

// Never obfuscate the FileFilter type.
var _ = reflect.TypeOf(FileFilter{})

type DialogConfig struct {
// The title of the dialog
Title string
Expand Down Expand Up @@ -69,13 +76,21 @@ func (config *DialogConfig) apply(dialog Dialog) (err error) {
}

if config.Folder != "" {
_, err = os.Stat(config.Folder)
if err != nil {
return
}
Comment on lines +79 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Path validation might be too strict for network paths.

The current implementation using os.Stat might fail for valid Windows UNC paths or network shares that are temporarily unavailable. This could prevent users from accessing network locations.

Consider using a more flexible path validation approach:

-		_, err = os.Stat(config.Folder)
-		if err != nil {
-			return
-		}
+		if !isValidPath(config.Folder) {
+			return fmt.Errorf("invalid folder path: %s", config.Folder)
+		}

Add this helper function:

func isValidPath(path string) bool {
    // Basic path validation
    if path == "" {
        return false
    }
    
    // Check if it's a Windows UNC path
    if strings.HasPrefix(path, "\\\\") {
        return true
    }
    
    // For local paths, check if they exist
    _, err := os.Stat(path)
    return err == nil
}

Also applies to: 90-93

err = dialog.SetFolder(config.Folder)
if err != nil {
return
}
}

if config.DefaultFolder != "" {
_, err = os.Stat(config.DefaultFolder)
if err != nil {
return
}
err = dialog.SetDefaultFolder(config.DefaultFolder)
if err != nil {
return
Expand Down
4 changes: 3 additions & 1 deletion v2/internal/go-common-file-dialog/cfd/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ package cfd

import "errors"

var ErrCancelled = errors.New("cancelled by user")
var (
ErrorCancelled = errors.New("cancelled by user")
)
10 changes: 7 additions & 3 deletions v2/internal/go-common-file-dialog/cfd/iFileOpenDialog.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ package cfd

import (
"github.com/go-ole/go-ole"
"github.com/wailsapp/wails/v2/internal/go-common-file-dialog/util"
"github.com/google/uuid"
"syscall"
"unsafe"
)
Expand Down Expand Up @@ -106,7 +106,7 @@ func (fileOpenDialog *iFileOpenDialog) SetFileFilters(filter []FileFilter) error
}

func (fileOpenDialog *iFileOpenDialog) SetRole(role string) error {
return fileOpenDialog.vtbl.setClientGuid(unsafe.Pointer(fileOpenDialog), util.StringToUUID(role))
return fileOpenDialog.vtbl.setClientGuid(unsafe.Pointer(fileOpenDialog), StringToUUID(role))
}

// This should only be callable when the user asks for a multi select because
Expand Down Expand Up @@ -177,7 +177,7 @@ func (vtbl *iFileOpenDialogVtbl) getResultsStrings(objPtr unsafe.Pointer) ([]str
return nil, err
}
if shellItemArray == nil {
return nil, ErrCancelled
return nil, ErrorCancelled
}
defer shellItemArray.vtbl.release(unsafe.Pointer(shellItemArray))
count, err := shellItemArray.vtbl.getCount(unsafe.Pointer(shellItemArray))
Expand All @@ -194,3 +194,7 @@ func (vtbl *iFileOpenDialogVtbl) getResultsStrings(objPtr unsafe.Pointer) ([]str
}
return results, nil
}

func StringToUUID(str string) *ole.GUID {
return ole.NewGUID(uuid.NewSHA1(uuid.Nil, []byte(str)).String())
}
3 changes: 1 addition & 2 deletions v2/internal/go-common-file-dialog/cfd/iFileSaveDialog.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package cfd

import (
"github.com/go-ole/go-ole"
"github.com/wailsapp/wails/v2/internal/go-common-file-dialog/util"
"unsafe"
)

Expand Down Expand Up @@ -77,7 +76,7 @@ func (fileSaveDialog *iFileSaveDialog) SetFileFilters(filter []FileFilter) error
}

func (fileSaveDialog *iFileSaveDialog) SetRole(role string) error {
return fileSaveDialog.vtbl.setClientGuid(unsafe.Pointer(fileSaveDialog), util.StringToUUID(role))
return fileSaveDialog.vtbl.setClientGuid(unsafe.Pointer(fileSaveDialog), StringToUUID(role))
}

func (fileSaveDialog *iFileSaveDialog) SetDefaultExtension(defaultExtension string) error {
Expand Down
3 changes: 2 additions & 1 deletion v2/internal/go-common-file-dialog/cfd/iShellItemArray.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package cfd

import (
"fmt"
"github.com/go-ole/go-ole"
"syscall"
"unsafe"
Expand Down Expand Up @@ -57,7 +58,7 @@ func (vtbl *iShellItemArrayVtbl) getItemAt(objPtr unsafe.Pointer, index uintptr)
return "", err
}
if shellItem == nil {
return "", ErrCancelled
return "", fmt.Errorf("shellItem is nil")
}
defer shellItem.vtbl.release(unsafe.Pointer(shellItem))
return shellItem.vtbl.getDisplayName(unsafe.Pointer(shellItem))
Expand Down
2 changes: 1 addition & 1 deletion v2/internal/go-common-file-dialog/cfd/vtblCommonFunc.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func (vtbl *iFileDialogVtbl) getResultString(objPtr unsafe.Pointer) (string, err
return "", err
}
if shellItem == nil {
return "", ErrCancelled
return "", fmt.Errorf("shellItem is nil")
}
defer shellItem.vtbl.release(unsafe.Pointer(shellItem))
return shellItem.vtbl.getDisplayName(unsafe.Pointer(shellItem))
Expand Down
16 changes: 4 additions & 12 deletions v2/internal/go-common-file-dialog/cfdutil/CFDUtil.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ func ShowOpenFileDialog(config cfd.DialogConfig) (string, error) {
if err != nil {
return "", err
}
defer func() {
_ = dialog.Release()
}()
defer dialog.Release()
return dialog.ShowAndGetResult()
}

Expand All @@ -22,9 +20,7 @@ func ShowOpenMultipleFilesDialog(config cfd.DialogConfig) ([]string, error) {
if err != nil {
return nil, err
}
defer func() {
_ = dialog.Release()
}()
defer dialog.Release()
return dialog.ShowAndGetResults()
}

Expand All @@ -34,9 +30,7 @@ func ShowPickFolderDialog(config cfd.DialogConfig) (string, error) {
if err != nil {
return "", err
}
defer func() {
_ = dialog.Release()
}()
defer dialog.Release()
return dialog.ShowAndGetResult()
}

Expand All @@ -46,8 +40,6 @@ func ShowSaveFileDialog(config cfd.DialogConfig) (string, error) {
if err != nil {
return "", err
}
defer func() {
_ = dialog.Release()
}()
defer dialog.Release()
return dialog.ShowAndGetResult()
}
1 change: 1 addition & 0 deletions website/src/pages/changelog.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fixed failed models.ts build due to non-json-encodable Go types [PR](https://github.com/wailsapp/wails/pull/3975) by [@pbnjay](https://github.com/pbnjay)
- Fixed more binding and typescript export bugs [PR](https://github.com/wailsapp/wails/pull/3978) by [@pbnjay](https://github.com/pbnjay)
- Fixed Dispatcher.ProcessMessage crash process instead of return error [PR](https://github.com/wailsapp/wails/pull/4016) [#4015](https://github.com/wailsapp/wails/issues/4015) by [@ronaldinho_x86](https://github.com/RonaldinhoL)
- Fixed Windows SaveDialog crash by [@leaanthony](https://github.com/leaanthony)

### Changed
- Allow to specify macos-min-version externally. Implemented by @APshenkin in [PR](https://github.com/wailsapp/wails/pull/3756)
Expand Down
Loading