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(net/ghttp): panic when post empty string parameter to *ghttp.UploadFile #4226

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

gqcn
Copy link
Member

@gqcn gqcn commented Mar 26, 2025

fixed #4193

@gqcn gqcn requested review from hailaz, oldme-git and wln32 March 26, 2025 13:33
@gqcn
Copy link
Member Author

gqcn commented Mar 26, 2025

related #4203

@hailaz
Copy link
Member

hailaz commented Mar 27, 2025

@gqcn 过不了单测

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@gqcn Can't pass the single test

@houseme houseme requested a review from Copilot March 28, 2025 03:52
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a panic when posting an empty string parameter to *ghttp.UploadFile by improving type conversion and error handling. Key changes include updates to conversion functions (struct, map, float), a modification of the default error handling in scanning, and the addition of a new test case for issue 4193.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
util/gconv/internal/converter/converter_struct.go Updated error message and early return when parameters cannot be converted to a struct pointer.
util/gconv/internal/converter/converter_map.go Added conditions to ensure an empty map is returned when required by the conversion options.
util/gconv/internal/converter/converter_float.go Added empty string checks to prevent parsing errors in float32 and float64 conversions.
util/gconv/internal/converter/converter_convert.go Refactored custom converter variable assignment and updated code comments.
util/gconv/gconv_scan.go Changed the Scan option default from ContinueOnError true to false.
net/ghttp/ghttp_z_unit_issue_test.go Added a new test case for issue 4193 to confirm correct behavior when an empty file parameter is posted.
Comments suppressed due to low confidence (3)

util/gconv/internal/converter/converter_struct.go:167

  • Consider using %v instead of %s for formatting pointerReflectValue.Type() to ensure the type is represented correctly.
"convert params \"%v\" to \"%s\" failed"

util/gconv/internal/converter/converter_float.go:75

  • The empty string check for variable s is performed twice in the Float32 conversion; consider removing the redundant check to avoid unnecessary computations.
if s == "" {

util/gconv/internal/converter/converter_float.go:149

  • The empty string check for variable s is redundantly performed in the Float64 conversion function; removing the duplicate check can simplify the logic.
if s == "" {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

net/ghttp: Multipart file upload fails: empty string causes ghttp.UploadFile conversion error
3 participants