This is an automated email from the ASF dual-hosted git repository.
laskoviymishka pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new 6cea8d48 fix(io): parse local file URIs consistently (#1578)
6cea8d48 is described below
commit 6cea8d4843026b76ade0cba3e2c13bfe0b98bc9a
Author: Minh Vu <[email protected]>
AuthorDate: Wed Aug 5 18:57:53 2026 +0200
fix(io): parse local file URIs consistently (#1578)
* fix(io): parse local file URIs consistently
Signed-off-by: Minh Vu <[email protected]>
* fix(io): preserve ordinary local path syntax
Signed-off-by: Minh Vu <[email protected]>
* fix(io): preserve colon-containing local paths
* fix(io): support Windows file URI paths
* fix(io): normalize encoded file URIs
---------
Signed-off-by: Minh Vu <[email protected]>
---
io/local.go | 166 ++++++++++++++++++++++++++++++++++++++++++-----
io/local_test.go | 120 +++++++++++++++++++++++++++++++---
io/local_windows_test.go | 45 +++++++++++++
3 files changed, 304 insertions(+), 27 deletions(-)
diff --git a/io/local.go b/io/local.go
index d511ba8a..e44669cc 100644
--- a/io/local.go
+++ b/io/local.go
@@ -18,7 +18,9 @@
package io
import (
+ "fmt"
"io/fs"
+ "net/url"
"os"
"path/filepath"
"strings"
@@ -28,16 +30,91 @@ import (
// the local file system.
type LocalFS struct{}
+func localPath(name string) (string, error) {
+ if filepath.VolumeName(name) != "" {
+ return name, nil
+ }
+
+ schemeEnd := strings.IndexByte(name, ':')
+ firstSeparator := strings.IndexAny(name, `/\`)
+ if schemeEnd < 0 || (firstSeparator >= 0 && firstSeparator < schemeEnd)
{
+ return name, nil
+ }
+ scheme := name[:schemeEnd]
+ // A colon is valid in a native POSIX filename. Treat non-file names as
+ // URIs only when they have an authority delimiter, preserving paths
such as
+ // "partition:2026/data.parquet" while still rejecting s3:// and similar
+ // locations passed directly to LocalFS.
+ if !strings.EqualFold(scheme, "file") &&
!strings.HasPrefix(name[schemeEnd+1:], "//") {
+ return name, nil
+ }
+ if !strings.EqualFold(scheme, "file") {
+ return "", fmt.Errorf("unsupported local filesystem scheme %q",
scheme)
+ }
+
+ parsed, err := url.Parse(name)
+ if err != nil {
+ return "", fmt.Errorf("invalid local file path %q: %w", name,
err)
+ }
+ if parsed.Host != "" && !strings.EqualFold(parsed.Host, "localhost") {
+ if filepath.Separator != '\\' ||
!isWindowsDriveHost(parsed.Host) {
+ return "", fmt.Errorf("unsupported file URI authority
%q", parsed.Host)
+ }
+ }
+
+ path := parsed.Path
+ if parsed.Opaque != "" {
+ var err error
+ path, err = url.PathUnescape(parsed.Opaque)
+ if err != nil {
+ return "", fmt.Errorf("invalid local file path %q: %w",
name, err)
+ }
+ }
+
+ if filepath.Separator == '\\' && isWindowsDriveHost(parsed.Host) {
+ path = parsed.Host + path
+ } else if filepath.Separator == '\\' && isWindowsDrivePath(path) {
+ path = path[1:]
+ }
+
+ return filepath.FromSlash(path), nil
+}
+
+func isWindowsDrivePath(path string) bool {
+ return len(path) >= 3 && path[0] == '/' &&
+ ((path[1] >= 'a' && path[1] <= 'z') || (path[1] >= 'A' &&
path[1] <= 'Z')) &&
+ path[2] == ':'
+}
+
+func isWindowsDriveHost(host string) bool {
+ return len(host) == 2 &&
+ ((host[0] >= 'a' && host[0] <= 'z') || (host[0] >= 'A' &&
host[0] <= 'Z')) &&
+ host[1] == ':'
+}
+
func (LocalFS) Open(name string) (File, error) {
- return os.Open(strings.TrimPrefix(name, "file://"))
+ path, err := localPath(name)
+ if err != nil {
+ return nil, err
+ }
+
+ return os.Open(path)
}
func (LocalFS) ReadFile(name string) ([]byte, error) {
- return os.ReadFile(strings.TrimPrefix(name, "file://"))
+ path, err := localPath(name)
+ if err != nil {
+ return nil, err
+ }
+
+ return os.ReadFile(path)
}
func (LocalFS) Create(name string) (FileWriter, error) {
- filename := strings.TrimPrefix(name, "file://")
+ filename, err := localPath(name)
+ if err != nil {
+ return nil, err
+ }
if err := os.MkdirAll(filepath.Dir(filename), 0o755); err != nil {
return nil, err
}
@@ -46,7 +123,10 @@ func (LocalFS) Create(name string) (FileWriter, error) {
}
func (LocalFS) WriteFile(name string, content []byte) error {
- filename := strings.TrimPrefix(name, "file://")
+ filename, err := localPath(name)
+ if err != nil {
+ return err
+ }
if err := os.MkdirAll(filepath.Dir(filename), 0o755); err != nil {
return err
}
@@ -55,48 +135,98 @@ func (LocalFS) WriteFile(name string, content []byte)
error {
}
func (LocalFS) Remove(name string) error {
- return os.Remove(strings.TrimPrefix(name, "file://"))
+ path, err := localPath(name)
+ if err != nil {
+ return err
+ }
+
+ return os.Remove(path)
}
func (LocalFS) RemoveAll(name string) error {
- return os.RemoveAll(strings.TrimPrefix(name, "file://"))
+ path, err := localPath(name)
+ if err != nil {
+ return err
+ }
+
+ return os.RemoveAll(path)
}
func (LocalFS) WalkDir(root string, fn fs.WalkDirFunc) error {
- return filepath.WalkDir(strings.TrimPrefix(root, "file://"), fn)
+ path, err := localPath(root)
+ if err != nil {
+ return err
+ }
+
+ return filepath.WalkDir(path, fn)
}
func (LocalFS) ReadDir(name string) ([]fs.DirEntry, error) {
- return os.ReadDir(strings.TrimPrefix(name, "file://"))
+ path, err := localPath(name)
+ if err != nil {
+ return nil, err
+ }
+
+ return os.ReadDir(path)
}
-func (LocalFS) MkdirAll(path string) error {
- return os.MkdirAll(strings.TrimPrefix(path, "file://"), 0o755)
+func (LocalFS) MkdirAll(name string) error {
+ path, err := localPath(name)
+ if err != nil {
+ return err
+ }
+
+ return os.MkdirAll(path, 0o755)
}
-func (LocalFS) Mkdir(path string) error {
- return os.Mkdir(strings.TrimPrefix(path, "file://"), 0o755)
+func (LocalFS) Mkdir(name string) error {
+ path, err := localPath(name)
+ if err != nil {
+ return err
+ }
+
+ return os.Mkdir(path, 0o755)
}
func (LocalFS) Stat(name string) (fs.FileInfo, error) {
- return os.Stat(strings.TrimPrefix(name, "file://"))
+ path, err := localPath(name)
+ if err != nil {
+ return nil, err
+ }
+
+ return os.Stat(path)
}
func (LocalFS) Rename(oldpath, newpath string) error {
- return os.Rename(strings.TrimPrefix(oldpath, "file://"),
strings.TrimPrefix(newpath, "file://"))
+ src, err := localPath(oldpath)
+ if err != nil {
+ return err
+ }
+ dst, err := localPath(newpath)
+ if err != nil {
+ return err
+ }
+
+ return os.Rename(src, dst)
}
func (LocalFS) RenameNoReplace(oldpath, newpath string) error {
- oldpath = strings.TrimPrefix(oldpath, "file://")
- newpath = strings.TrimPrefix(newpath, "file://")
+ src, err := localPath(oldpath)
+ if err != nil {
+ return err
+ }
+ dst, err := localPath(newpath)
+ if err != nil {
+ return err
+ }
- if err := os.Link(oldpath, newpath); err != nil {
+ if err = os.Link(src, dst); err != nil {
return err
}
// Publishing already succeeded once the hard link exists. Removing the
// source temp file is best-effort cleanup.
- _ = os.Remove(oldpath)
+ _ = os.Remove(src)
return nil
}
diff --git a/io/local_test.go b/io/local_test.go
index 2b1d3129..addf2225 100644
--- a/io/local_test.go
+++ b/io/local_test.go
@@ -19,10 +19,10 @@ package io
import (
"io/fs"
+ "net/url"
"os"
"path/filepath"
"sort"
- "strings"
"testing"
"github.com/stretchr/testify/assert"
@@ -73,7 +73,7 @@ func TestLocalFSWalkDirWithFileScheme(t *testing.T) {
lfs := LocalFS{}
var files []string
- err := lfs.WalkDir("file://"+dir, func(path string, d fs.DirEntry, err
error) error {
+ err := lfs.WalkDir(fileURI(dir), func(path string, d fs.DirEntry, err
error) error {
if err != nil {
return err
}
@@ -93,32 +93,134 @@ func TestLocalFSWriteFileCreatesParentDirectories(t
*testing.T) {
content := []byte("content")
for _, tt := range []struct {
- name string
- path string
+ name string
+ path string
+ readPath string
}{
{
- name: "plain path",
- path: filepath.Join(dir, "plain", "nested", "file.txt"),
+ name: "plain path",
+ path: filepath.Join(dir, "plain", "nested",
"file.txt"),
+ readPath: filepath.Join(dir, "plain", "nested",
"file.txt"),
},
{
- name: "file scheme",
- path: "file://" + filepath.Join(dir, "scheme",
"nested", "file.txt"),
+ name: "file scheme",
+ path: fileURI(filepath.Join(dir, "scheme",
"nested", "file.txt")),
+ readPath: filepath.Join(dir, "scheme", "nested",
"file.txt"),
+ },
+ {
+ name: "escaped file scheme",
+ path: fileURI(filepath.Join(dir, "scheme with
space", "nested", "file.txt")),
+ readPath: filepath.Join(dir, "scheme with space",
"nested", "file.txt"),
},
} {
t.Run(tt.name, func(t *testing.T) {
require.NoError(t, LocalFS{}.WriteFile(tt.path,
content))
- got, err := os.ReadFile(strings.TrimPrefix(tt.path,
"file://"))
+ got, err := os.ReadFile(tt.readPath)
require.NoError(t, err)
assert.Equal(t, content, got)
})
}
}
+func TestLocalFSParsesFileURIs(t *testing.T) {
+ t.Parallel()
+
+ dir := t.TempDir()
+ path := filepath.Join(dir, "metadata.json")
+ require.NoError(t, os.WriteFile(path, []byte("metadata"), 0o600))
+
+ for _, name := range []string{
+ path,
+ fileURI(path),
+ singleSlashFileURI(path),
+ "file:" + filepath.ToSlash(path),
+ localhostFileURI(path),
+ } {
+ content, err := (LocalFS{}).ReadFile(name)
+ require.NoError(t, err, name)
+ assert.Equal(t, []byte("metadata"), content)
+ }
+}
+
+func TestLocalFSParsesEscapedFileURIsConsistently(t *testing.T) {
+ t.Parallel()
+
+ dir := t.TempDir()
+ path := filepath.Join(dir, "metadata with space%20.txt")
+ require.NoError(t, os.WriteFile(path, []byte("metadata"), 0o600))
+
+ for _, name := range []string{
+ fileURI(path),
+ singleSlashFileURI(path),
+ } {
+ content, err := (LocalFS{}).ReadFile(name)
+ require.NoError(t, err, name)
+ assert.Equal(t, []byte("metadata"), content)
+ }
+}
+
+func TestLocalFSRejectsUnsupportedFileURIAuthority(t *testing.T) {
+ t.Parallel()
+
+ _, err := (LocalFS{}).Open("file://remotehost/tmp/metadata.json")
+ require.ErrorContains(t, err, `unsupported file URI authority
"remotehost"`)
+}
+
+func TestLocalFSPreservesPlainPathsWithRawPercent(t *testing.T) {
+ t.Parallel()
+
+ path := filepath.Join(t.TempDir(), "discount:100%off.txt")
+ require.NoError(t, os.WriteFile(path, []byte("content"), 0o600))
+
+ content, err := (LocalFS{}).ReadFile(path)
+ require.NoError(t, err)
+ assert.Equal(t, []byte("content"), content)
+}
+
+func TestLocalFSPreservesRelativePathsWithColon(t *testing.T) {
+ t.Parallel()
+
+ path := "partition:2026/data.parquet"
+ got, err := localPath(path)
+ require.NoError(t, err)
+ assert.Equal(t, path, got)
+}
+
func TestLocalFSImplementsListableIO(t *testing.T) {
var _ ListableIO = LocalFS{}
}
+func fileURI(path string) string {
+ hasVolume := filepath.VolumeName(path) != ""
+ path = filepath.ToSlash(path)
+ if hasVolume {
+ path = "/" + path
+ }
+
+ return (&url.URL{Scheme: "file", Path: path}).String()
+}
+
+func singleSlashFileURI(path string) string {
+ hasVolume := filepath.VolumeName(path) != ""
+ pathSlash := filepath.ToSlash(path)
+ if hasVolume {
+ pathSlash = "/" + pathSlash
+ }
+
+ return "file:" + (&url.URL{Path: pathSlash}).EscapedPath()
+}
+
+func localhostFileURI(path string) string {
+ hasVolume := filepath.VolumeName(path) != ""
+ path = filepath.ToSlash(path)
+ if hasVolume {
+ path = "/" + path
+ }
+
+ return (&url.URL{Scheme: "file", Host: "localhost", Path:
path}).String()
+}
+
func TestLocalFSRenameNoReplaceDoesNotOverwrite(t *testing.T) {
dir := t.TempDir()
src := filepath.Join(dir, "src")
diff --git a/io/local_windows_test.go b/io/local_windows_test.go
new file mode 100644
index 00000000..0215d747
--- /dev/null
+++ b/io/local_windows_test.go
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//go:build windows
+
+package io
+
+import (
+ "os"
+ "path/filepath"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestLocalFSParsesWindowsDriveFileURIs(t *testing.T) {
+ path := filepath.Join(t.TempDir(), "metadata.json")
+ require.NoError(t, os.WriteFile(path, []byte("metadata"), 0o600))
+
+ pathSlash := filepath.ToSlash(path)
+ for _, name := range []string{
+ "file://" + pathSlash,
+ "file:///" + pathSlash,
+ "file:/" + pathSlash,
+ "file://localhost/" + pathSlash,
+ } {
+ content, err := (LocalFS{}).ReadFile(name)
+ require.NoError(t, err, name)
+ require.Equal(t, []byte("metadata"), content)
+ }
+}
