From 5230f200ced0694b6d7f7115edecc95e693524d3 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 11 Jul 2023 15:07:53 +0100 Subject: [PATCH 1/2] filesync: escape special query characters This ensures that files with '%' and '+' can still be properly copied. To prevent regressions, this also adds in a couple of example test cases. Signed-off-by: Justin Chadwell --- frontend/dockerfile/dockerfile_test.go | 16 ++++++++++++++-- session/filesync/filesync.go | 5 +++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index 47e7339516d8..ea47dfc4d975 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -6766,12 +6766,16 @@ func testCopyUnicodePath(t *testing.T, sb integration.Sandbox) { dockerfile := []byte(` FROM alpine COPY test-äöü.txt / +COPY test-%C3%A4%C3%B6%C3%BC.txt / +COPY test+aou.txt / `) dir, err := integration.Tmpdir( t, fstest.CreateFile("Dockerfile", dockerfile, 0600), - fstest.CreateFile("test-äöü.txt", []byte("test"), 0644), + fstest.CreateFile("test-äöü.txt", []byte("foo"), 0644), + fstest.CreateFile("test-%C3%A4%C3%B6%C3%BC.txt", []byte("bar"), 0644), + fstest.CreateFile("test+aou.txt", []byte("baz"), 0644), ) require.NoError(t, err) @@ -6794,7 +6798,15 @@ COPY test-äöü.txt / dt, err := os.ReadFile(filepath.Join(destDir, "test-äöü.txt")) require.NoError(t, err) - require.Equal(t, "test", string(dt)) + require.Equal(t, "foo", string(dt)) + + dt, err = os.ReadFile(filepath.Join(destDir, "test-%C3%A4%C3%B6%C3%BC.txt")) + require.NoError(t, err) + require.Equal(t, "bar", string(dt)) + + dt, err = os.ReadFile(filepath.Join(destDir, "test+aou.txt")) + require.NoError(t, err) + require.Equal(t, "baz", string(dt)) } func runShell(dir string, cmds ...string) error { diff --git a/session/filesync/filesync.go b/session/filesync/filesync.go index 522f453203b4..e2a5ce8f2645 100644 --- a/session/filesync/filesync.go +++ b/session/filesync/filesync.go @@ -387,8 +387,9 @@ func decodeOpts(opts map[string][]string) map[string][]string { func encodeStringForHeader(input string) string { var output strings.Builder for _, runeVal := range input { - // Only encode non-ASCII characters. - if runeVal > unicode.MaxASCII { + // Only encode non-ASCII characters, and characters that have special + // meaning during decoding. + if runeVal > unicode.MaxASCII || runeVal == '%' || runeVal == '+' { // Encode each non-ASCII character individually. output.WriteString(url.QueryEscape(string(runeVal))) } else { From 48b62404e37fd060d3a0b05e3de58e4c69256f83 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 12 Jul 2023 09:23:42 -0700 Subject: [PATCH 2/2] filesync: fix backward compatibility with encoding + and % Signed-off-by: Tonis Tiigi --- session/filesync/filesync.go | 67 ++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/session/filesync/filesync.go b/session/filesync/filesync.go index e2a5ce8f2645..712469a9b779 100644 --- a/session/filesync/filesync.go +++ b/session/filesync/filesync.go @@ -27,7 +27,6 @@ const ( keyFollowPaths = "followpaths" keyDirName = "dir-name" keyExporterMetaPrefix = "exporter-md-" - keyOptsEncoded = "opts-encoded" ) type fsSyncProvider struct { @@ -86,17 +85,7 @@ func (sp *fsSyncProvider) handle(method string, stream grpc.ServerStream) (retEr } opts, _ := metadata.FromIncomingContext(stream.Context()) // if no metadata continue with empty object - - isDecoded := false - if v, ok := opts[keyOptsEncoded]; ok && len(v) > 0 { - if b, _ := strconv.ParseBool(v[0]); b { - isDecoded = true - } - } - - if isDecoded { - opts = decodeOpts(opts) - } + opts = decodeOpts(opts) dirName := "" name, ok := opts[keyDirName] @@ -224,9 +213,6 @@ func FSSync(ctx context.Context, c session.Caller, opt FSSendRequestOpt) error { var stream grpc.ClientStream - // mark that we have encoded options so older versions with raw values can be detected on client side - opts[keyOptsEncoded] = []string{"1"} - opts = encodeOpts(opts) ctx = metadata.NewOutgoingContext(ctx, opts) @@ -361,11 +347,11 @@ func (e InvalidSessionError) Unwrap() error { func encodeOpts(opts map[string][]string) map[string][]string { md := make(map[string][]string, len(opts)) for k, v := range opts { - out := make([]string, len(v)) - for i, s := range v { - out[i] = encodeStringForHeader(s) - } + out, encoded := encodeStringForHeader(v) md[k] = out + if encoded { + md[k+"-encoded"] = []string{"1"} + } } return md } @@ -374,8 +360,18 @@ func decodeOpts(opts map[string][]string) map[string][]string { md := make(map[string][]string, len(opts)) for k, v := range opts { out := make([]string, len(v)) - for i, s := range v { - out[i], _ = url.QueryUnescape(s) + var isDecoded bool + if v, ok := opts[k+"-encoded"]; ok && len(v) > 0 { + if b, _ := strconv.ParseBool(v[0]); b { + isDecoded = true + } + } + if isDecoded { + for i, s := range v { + out[i], _ = url.QueryUnescape(s) + } + } else { + copy(out, v) } md[k] = out } @@ -384,18 +380,23 @@ func decodeOpts(opts map[string][]string) map[string][]string { // encodeStringForHeader encodes a string value so it can be used in grpc header. This encoding // is backwards compatible and avoids encoding ASCII characters. -func encodeStringForHeader(input string) string { - var output strings.Builder - for _, runeVal := range input { - // Only encode non-ASCII characters, and characters that have special - // meaning during decoding. - if runeVal > unicode.MaxASCII || runeVal == '%' || runeVal == '+' { - // Encode each non-ASCII character individually. - output.WriteString(url.QueryEscape(string(runeVal))) - } else { - // Directly append ASCII characters and '*' to the output. - output.WriteRune(runeVal) +func encodeStringForHeader(inputs []string) ([]string, bool) { + var encode bool + for _, input := range inputs { + for _, runeVal := range input { + // Only encode non-ASCII characters, and characters that have special + // meaning during decoding. + if runeVal > unicode.MaxASCII { + encode = true + break + } } } - return output.String() + if !encode { + return inputs, false + } + for i, input := range inputs { + inputs[i] = url.QueryEscape(input) + } + return inputs, true }