Apply url.QueryUnescape for get image by seokho-son · Pull Request #2130 · cloud-barista/cb-tumblebug

seokho-son · 2025-09-01T18:47:00Z

related with issue [Azure, Alibaba] There may be issues with image initialization or retrieving image information. cm-beetle#194

But, I will double check :)

Signed-off-by: seokho-son <shsongist@gmail.com>

Copilot

Pull Request Overview

This PR improves URL-encoded parameter handling in the image retrieval endpoint by replacing manual URL decoding with Go's built-in url.QueryUnescape function. The change addresses issue #194 related to properly decoding URL-encoded characters in image IDs.

Replaces manual %2B to + conversion with comprehensive URL decoding using url.QueryUnescape
Adds fallback manual decoding for common URL-encoded characters when the built-in function fails
Updates API documentation to clarify accepted imageId parameter formats
Includes response body truncation functionality for debug logging

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/interface/rest/server/resource/image.go	Implements proper URL decoding for image keys with fallback handling
src/interface/rest/docs/swagger.yaml	Updates API documentation descriptions for image parameter formats
src/interface/rest/docs/swagger.json	Updates generated swagger JSON documentation
src/interface/rest/docs/docs.go	Updates generated Go documentation template
src/core/common/client/client.go	Adds response body truncation for debug logging (unrelated to main PR purpose)

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-01T18:48:04Z

src/interface/rest/server/resource/image.go

+		log.Warn().Err(err).Msgf("Failed to URL decode imageKey: %s, using original", imageKey)
+		// Fall back to manual decoding for common characters
+		imageKey = strings.ReplaceAll(imageKey, "%2B", "+")  // +
+		imageKey = strings.ReplaceAll(imageKey, "%3A", ":")  // :
+		imageKey = strings.ReplaceAll(imageKey, "%2F", "/")  // /
+		imageKey = strings.ReplaceAll(imageKey, "%3F", "?")  // ?
+		imageKey = strings.ReplaceAll(imageKey, "%23", "#")  // #
+		imageKey = strings.ReplaceAll(imageKey, "%26", "&")  // &
+		imageKey = strings.ReplaceAll(imageKey, "%3D", "=")  // =
+		imageKey = strings.ReplaceAll(imageKey, "%25", "%")  // % (handle this last to avoid double decoding)
+		imageKey = strings.ReplaceAll(imageKey, "%40", "@")  // @
+		imageKey = strings.ReplaceAll(imageKey, "%21", "!")  // !
+		imageKey = strings.ReplaceAll(imageKey, "%2A", "*")  // *
+		imageKey = strings.ReplaceAll(imageKey, "%28", "(")  // (
+		imageKey = strings.ReplaceAll(imageKey, "%29", ")")  // )
+		imageKey = strings.ReplaceAll(imageKey, "%5B", "[")  // [
+		imageKey = strings.ReplaceAll(imageKey, "%5D", "]")  // ]
+		imageKey = strings.ReplaceAll(imageKey, "%7B", "{")  // {
+		imageKey = strings.ReplaceAll(imageKey, "%7D", "}")  // }
+		imageKey = strings.ReplaceAll(imageKey, "%7C", "|")  // |
+		imageKey = strings.ReplaceAll(imageKey, "%5C", "\\") // \
+		imageKey = strings.ReplaceAll(imageKey, "%5E", "^")  // ^
+		imageKey = strings.ReplaceAll(imageKey, "%7E", "~")  // ~
+		imageKey = strings.ReplaceAll(imageKey, "%60", "`")  // `
+		imageKey = strings.ReplaceAll(imageKey, "%20", " ")  // space
+	} else {
+		imageKey = decodedImageKey
+	}


This extensive fallback manual decoding logic is complex and duplicates functionality already available in the standard library. Consider investigating why url.QueryUnescape would fail for valid URL-encoded strings, as this should handle all these cases. If specific edge cases exist that require fallback, document them clearly and simplify the fallback to only handle those specific cases.

Suggested change

log.Warn().Err(err).Msgf("Failed to URL decode imageKey: %s, using original", imageKey)

// Fall back to manual decoding for common characters

imageKey = strings.ReplaceAll(imageKey, "%2B", "+") // +

imageKey = strings.ReplaceAll(imageKey, "%3A", ":") // :

imageKey = strings.ReplaceAll(imageKey, "%2F", "/") // /

imageKey = strings.ReplaceAll(imageKey, "%3F", "?") // ?

imageKey = strings.ReplaceAll(imageKey, "%23", "#") // #

imageKey = strings.ReplaceAll(imageKey, "%26", "&") // &

imageKey = strings.ReplaceAll(imageKey, "%3D", "=") // =

imageKey = strings.ReplaceAll(imageKey, "%25", "%") // % (handle this last to avoid double decoding)

imageKey = strings.ReplaceAll(imageKey, "%40", "@") // @

imageKey = strings.ReplaceAll(imageKey, "%21", "!") // !

imageKey = strings.ReplaceAll(imageKey, "%2A", "*") // *

imageKey = strings.ReplaceAll(imageKey, "%28", "(") // (

imageKey = strings.ReplaceAll(imageKey, "%29", ")") // )

imageKey = strings.ReplaceAll(imageKey, "%5B", "[") // [

imageKey = strings.ReplaceAll(imageKey, "%5D", "]") // ]

imageKey = strings.ReplaceAll(imageKey, "%7B", "{") // {

imageKey = strings.ReplaceAll(imageKey, "%7D", "}") // }

imageKey = strings.ReplaceAll(imageKey, "%7C", "|") // |

imageKey = strings.ReplaceAll(imageKey, "%5C", "\\") // \

imageKey = strings.ReplaceAll(imageKey, "%5E", "^") // ^

imageKey = strings.ReplaceAll(imageKey, "%7E", "~") // ~

imageKey = strings.ReplaceAll(imageKey, "%60", "`") // `

imageKey = strings.ReplaceAll(imageKey, "%20", " ") // space

} else {

imageKey = decodedImageKey

}

log.Error().Err(err).Msgf("Failed to URL decode imageKey: %s", imageKey)

return echo.NewHTTPError(400, fmt.Sprintf("Invalid imageKey encoding: %s", imageKey))

}

imageKey = decodedImageKey

ok. I will trust url.QueryUnescape without fallback. :)

Copilot · 2025-09-01T18:48:05Z

src/interface/rest/docs/swagger.yaml

@@ -6567,7 +6567,7 @@ paths:
        in: path
        description: "(Note: imageId param will be refined in next release, enabled\
          \ for temporal support) This param accepts vaious input types as Image Key:\


There's a typo in the documentation: 'vaious' should be 'various'.

Suggested change

\ for temporal support) This param accepts vaious input types as Image Key:\

\ for temporal support) This param accepts various input types as Image Key:\

Copilot · 2025-09-01T18:48:05Z

src/interface/rest/docs/swagger.json

                    {
                        "type": "string",
-                        "description": "(Note: imageId param will be refined in next release, enabled for temporal support) This param accepts vaious input types as Image Key: cspImageName",
+                        "description": "(Note: imageId param will be refined in next release, enabled for temporal support) This param accepts vaious input types as Image Key: 1) provider+imageId, 2) provider+region+imageId, 3) imageId",


There's a typo in the documentation: 'vaious' should be 'various'.

Suggested change

"description": "(Note: imageId param will be refined in next release, enabled for temporal support) This param accepts vaious input types as Image Key: 1) provider+imageId, 2) provider+region+imageId, 3) imageId",

"description": "(Note: imageId param will be refined in next release, enabled for temporal support) This param accepts various input types as Image Key: 1) provider+imageId, 2) provider+region+imageId, 3) imageId",

Copilot · 2025-09-01T18:48:06Z

src/interface/rest/docs/docs.go

                    {
                        "type": "string",
-                        "description": "(Note: imageId param will be refined in next release, enabled for temporal support) This param accepts vaious input types as Image Key: cspImageName",
+                        "description": "(Note: imageId param will be refined in next release, enabled for temporal support) This param accepts vaious input types as Image Key: 1) provider+imageId, 2) provider+region+imageId, 3) imageId",


There's a typo in the documentation: 'vaious' should be 'various'.

Suggested change

"description": "(Note: imageId param will be refined in next release, enabled for temporal support) This param accepts vaious input types as Image Key: 1) provider+imageId, 2) provider+region+imageId, 3) imageId",

"description": "(Note: imageId param will be refined in next release, enabled for temporal support) This param accepts various input types as Image Key: 1) provider+imageId, 2) provider+region+imageId, 3) imageId",

seokho-son · 2025-09-02T02:53:34Z

Alibaba

http://localhost:1323/tumblebug/ns/system/resources/image/alibaba+ubuntu_22_04_x64_20G_alibase_20250722.vhd
- 추천 포맷 (ubuntu_22_04_x64_20G_alibase_20250722.vhd imageId 혼자는 프라이머리 키가 아님 provider 가 포함되어야 명확
http://localhost:1323/tumblebug/ns/system/resources/image/ubuntu_22_04_x64_20G_alibase_20250722.vhd
- 여러개가 검출된다면, 최상위 1개만 제공해주는 것
http://localhost:1323/tumblebug/ns/system/resources/image/alibaba+anyregionbutnotusedactually+ubuntu_22_04_x64_20G_alibase_20250722.vhd
- Spec 과 형태를 맞추기 위한 폼. 실제로 검색에 region은 사용하지 않음.

Azure

AWS

http://localhost:1323/tumblebug/ns/system/resources/image/aws+ami-02835aed2a5cb1d2a

Signed-off-by: Seokho Son <shsongist@gmail.com>

seokho-son · 2025-09-02T02:57:46Z

/approve

Apply url.QueryUnescape for get image

42d0098

Signed-off-by: seokho-son <shsongist@gmail.com>

Copilot AI review requested due to automatic review settings September 1, 2025 18:47

seokho-son requested a review from yunkon-kim as a code owner September 1, 2025 18:47

Copilot AI reviewed Sep 1, 2025

View reviewed changes

Get Image with verified test results

d4dfee8

Signed-off-by: Seokho Son <shsongist@gmail.com>

github-actions bot approved these changes Sep 2, 2025

View reviewed changes

github-actions bot added the approved This PR is approved and will be merged soon. label Sep 2, 2025

cb-github-robot merged commit aaeaa4e into cloud-barista:main Sep 2, 2025
6 checks passed

seokho-son mentioned this pull request Sep 2, 2025

[Azure, Alibaba] There may be issues with image initialization or retrieving image information. cloud-barista/cm-beetle#194

Closed

yunkon-kim mentioned this pull request Sep 2, 2025

Handle potential future changes to the imageKey (i.e., imageId) format cloud-barista/cm-beetle#195

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply url.QueryUnescape for get image#2130

Apply url.QueryUnescape for get image#2130
cb-github-robot merged 2 commits intocloud-barista:mainfrom
seokho-son:main

seokho-son commented Sep 1, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 1, 2025

Uh oh!

seokho-son Sep 2, 2025

Uh oh!

Copilot AI Sep 1, 2025

Uh oh!

Copilot AI Sep 1, 2025

Uh oh!

Copilot AI Sep 1, 2025

Uh oh!

seokho-son commented Sep 2, 2025

Uh oh!

seokho-son commented Sep 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	\ for temporal support) This param accepts vaious input types as Image Key:\
	\ for temporal support) This param accepts various input types as Image Key:\

	"description": "(Note: imageId param will be refined in next release, enabled for temporal support) This param accepts vaious input types as Image Key: 1) provider+imageId, 2) provider+region+imageId, 3) imageId",
	"description": "(Note: imageId param will be refined in next release, enabled for temporal support) This param accepts various input types as Image Key: 1) provider+imageId, 2) provider+region+imageId, 3) imageId",

Conversation

seokho-son commented Sep 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

seokho-son Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

seokho-son commented Sep 2, 2025

Uh oh!

seokho-son commented Sep 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants