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 a bug when binding to pointer #1448

Merged
merged 1 commit into from Mar 6, 2022
Merged

Conversation

golddranks
Copy link

@golddranks golddranks commented May 7, 2019

Hi, this one-line PR fixes a problem when binding to a pointer type and the bind fails. At the moment it tries to set to an incompatible type. Here's example code:

	var threshold float64
	var pThreshold *float64
	c.Params.Bind(&pThreshold, "threshold")
	if pThreshold != nil {
		threshold = *pThreshold
	}
	if pThreshold == nil || threshold < 0.0 || threshold > 1.0 {
		_, threshold, err = selectedModel.GetDefaultThreshold()
	}
	if err != nil {
		return c.RenderError(err)
	}

If the param is something that isn't convertible into a float, the return value of the Bind call in binder.go:160 isn't CanAddr(), and it tries to return the value itself, instead of a pointer. But because it is a pointer binder, it is supposed to return a pointer. Returning something else will cause it to crash with "can't set a float64 into a variable of type *float64" Fixing it to return a null pointer fixes the problem.

There is another, API problem with the binder API though. Bindings can fail, but at the moment, there isn't good ways to detect this failure. It would be great if the Bind call would return a boolean that tells whether the binding succeeded or not. As you can see, I'm binding here a float64 type, but I'm using a pointer as a stop-gap resolution to detect when the binding fails.

@notzippy notzippy changed the base branch from master to develop March 6, 2022 16:38
@notzippy notzippy merged commit d202b93 into revel:develop Mar 6, 2022
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.

None yet

2 participants