Repository navigation
Conversation
…econfig The handlers accepted any method and had no origin check, so any web page open in the user's browser could add, overwrite or delete saved configurations while the web UI was running. Only accept the method the UI uses and apply http.CrossOriginProtection.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces cross-origin request protection and HTTP method validation for the /saveconfig and /deleteconfig endpoints using a new configHandler wrapper, along with corresponding unit tests. However, a compilation error was identified because http.NewCrossOriginProtection() is called on the standard library's net/http package instead of the internal custom http package.
| // that other web sites open in the user's browser cannot change or delete | ||
| // saved configurations. | ||
| func configHandler(method string, h http.HandlerFunc) http.Handler { | ||
| return http.NewCrossOriginProtection().Handler(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { |
There was a problem hiding this comment.
The code uses http.NewCrossOriginProtection(), but the http package imported in this file is "net/http" from the standard library, which does not contain this function. This will cause a compilation error.\n\nTo fix this, you need to import the custom http package from github.com/google/pprof/internal/http under a different name (e.g., pprofhttp) to avoid conflicting with "net/http", and then use pprofhttp.NewCrossOriginProtection() here.\n\nFor example, add this to your imports:\ngo\npprofhttp \"github.com/google/pprof/internal/http\"\n
\treturn pprofhttp.NewCrossOriginProtection().Handler(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {There was a problem hiding this comment.
This one is a false positive. http.NewCrossOriginProtection is in the standard net/http since Go 1.25, and go.mod already requires 1.26. There is no internal/http package in this repo. The branch builds and TestConfigHandlersRejectCrossOrigin passes.
The
/saveconfigand/deleteconfighandlers write to the settings file underos.UserConfigDir()but accept any method and never check the origin, so whilepprof -httpis running any page open in the same browser can add, overwrite or delete saved configs with<img src="http://localhost:PORT/ui/saveconfig?config=x&focus=...">or a no-corsfetch. The request comes from the user's own browser withHost: localhost, so the local-client check indefaultWebServerdoes not help. The handlers now only accept the method the UI already sends (POST for save, DELETE for delete) and are wrapped inhttp.CrossOriginProtection, andTestConfigHandlersRejectCrossOriginfails without the change.