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 all commits
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
2 changes: 1 addition & 1 deletion .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-22.04, windows-latest, macos-latest, ubuntu-24.04]
go-version: ['1.21']
go-version: ['1.23']
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

Invalid Go version specified

The specified Go version '1.23' is not yet released and may not be available. This could cause the workflow to fail.

Use a currently available Go version:

-        go-version: ['1.23']
+        go-version: ['1.21']
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
go-version: ['1.23']
go-version: ['1.21']


steps:
- name: Checkout code
Expand Down
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