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.

Sign up for GitHub

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

Jump to bottom

fix: infinite recursion bug #20862

Merged
merged 2 commits into from
Mar 4, 2021
Merged

fix: infinite recursion bug #20862

merged 2 commits into from
Mar 4, 2021

Conversation

lesam
Copy link
Contributor

@lesam lesam commented Mar 4, 2021

Closes #20249

We should avoid infinite recursion!

@lesam lesam requested a review from davidby-influx March 4, 2021 20:21
lesam
lesam commented Mar 4, 2021
View reviewed changes
services/httpd/response_writer.go
writer := bytesCountWriter{w: w.ResponseWriter}
err := w.formatter.WriteResponse(&writer, resp)
if err != nil {
n, _ = WriteError(w, err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

WriteError calls WriteResponse, and if the ResponseWriter is bad and returning errors, that calls back to WriteError. This was introduced recently, never released.

davidby-influx
davidby-influx previously approved these changes Mar 4, 2021
View reviewed changes
Copy link
Contributor

@davidby-influx davidby-influx left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@lesam lesam requested a review from davidby-influx March 4, 2021 20:34
davidby-influx
davidby-influx previously approved these changes Mar 4, 2021
View reviewed changes
Copy link
Contributor

@davidby-influx davidby-influx left a comment

Choose a reason for hiding this comment

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

One comment to ponder, but not blocking.

services/httpd/response_writer.go Show resolved Hide resolved
@lesam
fix: No infinite recursion on write error
2534679 
If there is some error writing to the response writer, we
would previous have infinite recursion.

Re-closes influxdata#20249
@lesam lesam requested a review from davidby-influx March 4, 2021 20:47
davidby-influx
davidby-influx approved these changes Mar 4, 2021
View reviewed changes
Copy link
Contributor

@davidby-influx davidby-influx left a comment

Choose a reason for hiding this comment

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

All the right errors returned, all the wrong errors ignored!

@danxmoran danxmoran mentioned this pull request Mar 4, 2021
Open
@lesam lesam merged commit d6f7716 into influxdata:master-1.x Mar 4, 2021
davidby-influx pushed a commit that referenced this pull request Mar 6, 2021
@lesam @davidby-influx
fix: infinite recursion bug (#20862)
d1929f5 
* Revert "fix(error): unsupported value: +Inf" error not handled gracefully (#20250)"

This reverts commit 6ac0bb3.

* fix: No infinite recursion on write error

If there is some error writing to the response writer, we
would previous have infinite recursion.

Re-closes #20249

(cherry picked from commit d6f7716)
davidby-influx pushed a commit that referenced this pull request Mar 10, 2021
@lesam @davidby-influx
fix: infinite recursion bug (#20862)
d497406 
* Revert "fix(error): unsupported value: +Inf" error not handled gracefully (#20250)"

This reverts commit 6ac0bb3.

* fix: No infinite recursion on write error

If there is some error writing to the response writer, we
would previous have infinite recursion.

Re-closes #20249

(cherry picked from commit d6f7716)
davidby-influx added a commit that referenced this pull request Mar 10, 2021
@davidby-influx @lesam
fix: infinite recursion bug (#20862) (#20914)
8a31b0e 
* fix: infinite recursion bug (#20862)

* Revert "fix(error): unsupported value: +Inf" error not handled gracefully (#20250)"

This reverts commit 6ac0bb3.

* fix: No infinite recursion on write error

If there is some error writing to the response writer, we
would previous have infinite recursion.

Re-closes #20249

(cherry picked from commit d6f7716)

* chore: update CHANGELOG.md

Co-authored-by: Sam Arnold <sarnold@influxdata.com>
been2io pushed a commit to been2io/influxdb that referenced this pull request Apr 1, 2021
Merge branch '1.8' of github.com:influxdata/influxdb into 1.8
b12e75d 
* '1.8' of github.com:influxdata/influxdb: (41 commits)
  chore: Late to the party fix for influxdata/plutonium#3339 (influxdata#21080)
  fix: fix help test for influx_inspect (influxdata#21052) (influxdata#21053)
  fix: Set go version to 1.13 in go.mod; see influxdata/plutonium#3339 (influxdata#21034)
  refactor: separate coarse and fine permission interfaces (influxdata#20996) (influxdata#21035)
  feat: Log query text for POST requests (influxdata#20993) (influxdata#21021)
  feat: influx_inspect export to standard out (influxdata#20977) (influxdata#20989)
  feat(inspect): Add report-disk for disk usage by measurement (influxdata#20917)
  fix: infinite recursion bug (influxdata#20862) (influxdata#20914)
  fix(tsdb): minimize lock contention when adding new fields or measure (influxdata#20912)
  fix(tsm1): fix data race when accessing tombstone stats (influxdata#20909)
  Update changelog
  feat: Make meta queries respect QueryTimeout values (influxdata#20910)
  chore: run goimports on 1.8 branch to bring it up to new check-in standards (influxdata#20907)
  fix(error): SELECT INTO doesn't return error with unsupported value (influxdata#20429) (influxdata#20432)
  build: switch tested centos base images (influxdata#20417) (influxdata#20418)
  chore: update CHANGELOG.md for typo and community PR (influxdata#20389)
  fix(prometheus): regexp handling should comply with PromQL (influxdata#19832)
  fix: cp.Mux.Serve() closes all net.Listener instances silently on error (influxdata#20295)
  Update changelog
  chore: fix CHANGELOG formating (influxdata#20286)
  ...
been2io pushed a commit to been2io/influxdb that referenced this pull request Apr 2, 2021
Merge branch '1.8' into seriescache
da59474 
* 1.8: (41 commits)
  chore: Late to the party fix for influxdata/plutonium#3339 (influxdata#21080)
  fix: fix help test for influx_inspect (influxdata#21052) (influxdata#21053)
  fix: Set go version to 1.13 in go.mod; see influxdata/plutonium#3339 (influxdata#21034)
  refactor: separate coarse and fine permission interfaces (influxdata#20996) (influxdata#21035)
  feat: Log query text for POST requests (influxdata#20993) (influxdata#21021)
  feat: influx_inspect export to standard out (influxdata#20977) (influxdata#20989)
  feat(inspect): Add report-disk for disk usage by measurement (influxdata#20917)
  fix: infinite recursion bug (influxdata#20862) (influxdata#20914)
  fix(tsdb): minimize lock contention when adding new fields or measure (influxdata#20912)
  fix(tsm1): fix data race when accessing tombstone stats (influxdata#20909)
  Update changelog
  feat: Make meta queries respect QueryTimeout values (influxdata#20910)
  chore: run goimports on 1.8 branch to bring it up to new check-in standards (influxdata#20907)
  fix(error): SELECT INTO doesn't return error with unsupported value (influxdata#20429) (influxdata#20432)
  build: switch tested centos base images (influxdata#20417) (influxdata#20418)
  chore: update CHANGELOG.md for typo and community PR (influxdata#20389)
  fix(prometheus): regexp handling should comply with PromQL (influxdata#19832)
  fix: cp.Mux.Serve() closes all net.Listener instances silently on error (influxdata#20295)
  Update changelog
  chore: fix CHANGELOG formating (influxdata#20286)
  ...

# Conflicts:
#	cmd/influxd/run/server.go
#	go.sum
#	services/httpd/service.go
#	storage/reads/datatypes/predicate.pb.go
#	storage/reads/datatypes/storage_common.pb.go
been2io pushed a commit to been2io/influxdb that referenced this pull request Apr 2, 2021
Merge branch 'seriescache' into seriesids
fd04807 
* seriescache: (41 commits)
  chore: Late to the party fix for influxdata/plutonium#3339 (influxdata#21080)
  fix: fix help test for influx_inspect (influxdata#21052) (influxdata#21053)
  fix: Set go version to 1.13 in go.mod; see influxdata/plutonium#3339 (influxdata#21034)
  refactor: separate coarse and fine permission interfaces (influxdata#20996) (influxdata#21035)
  feat: Log query text for POST requests (influxdata#20993) (influxdata#21021)
  feat: influx_inspect export to standard out (influxdata#20977) (influxdata#20989)
  feat(inspect): Add report-disk for disk usage by measurement (influxdata#20917)
  fix: infinite recursion bug (influxdata#20862) (influxdata#20914)
  fix(tsdb): minimize lock contention when adding new fields or measure (influxdata#20912)
  fix(tsm1): fix data race when accessing tombstone stats (influxdata#20909)
  Update changelog
  feat: Make meta queries respect QueryTimeout values (influxdata#20910)
  chore: run goimports on 1.8 branch to bring it up to new check-in standards (influxdata#20907)
  fix(error): SELECT INTO doesn't return error with unsupported value (influxdata#20429) (influxdata#20432)
  build: switch tested centos base images (influxdata#20417) (influxdata#20418)
  chore: update CHANGELOG.md for typo and community PR (influxdata#20389)
  fix(prometheus): regexp handling should comply with PromQL (influxdata#19832)
  fix: cp.Mux.Serve() closes all net.Listener instances silently on error (influxdata#20295)
  Update changelog
  chore: fix CHANGELOG formating (influxdata#20286)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidby-influx davidby-influx davidby-influx approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lesam @davidby-influx