Our context package predates the establishment of current best practices
regarding context usage and it shows. It encourages bad practices such
as using contexts to propagate non-request-scoped values like the
application version and using string-typed keys for context values. Move
the package internal to remove it from the API surface of
distribution/v3@v3.0.0 so we are free to iterate on it without being
constrained by compatibility.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Instead, provide a variant of instrumentedResponseWriter that does not
implement CloseNotifier, and use that when necessary. In
copyFullPayload, log instead of panicing when we encounter something
that doesn't implement CloseNotifier.
This is more complicated than I'd like, but it's necessary because
instrumentedResponseWriter must not embed CloseNotifier unless there's
really a CloseNotifier to embed.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
This channel never gets written to, but this only means that the mock
ResponseWriter will never signal a premature disconnect.
Based on feedback from #763.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
When a client disconnects without completing a HTTP request, we were
attempting to process the partial request, which usually leads to a 400
error. These errors can pollute the logs and make it more difficult to
track down real bugs.
This change uses CloseNotifier to detect disconnects. In combination
with checking Content-Length, we can detect a disconnect before sending
the full payload, and avoid logging a 400 error.
This logic is only applied to PUT, POST, and PATCH endpoints, as these
are the places where disconnects during a request are most likely to
happen.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
This is ensures that users of the ResponseWriter from the context correctly
track usage. Otherwise, context reporting is incorrect.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
You shouldn't have to import both:
github.com/docker/distribution/context
golang.org/x/net/context
just to use the distribution tools and implement the distribution interfaces.
By pulling the Context interface from golang.org/x/net/context into the
context package within the distribution project, you no longer have to import
both packages.
Note: You do not have to change anything anywhere else yet! All current uses
of both packages together will still work correctly because the Context
interface from either package is identical.
I've also made some other minor changes:
- Added a RemoteIP function. It's like RemoteAddr but discards the port suffix
- Added `.String()` to the response duration context value so that JSON log
formatting shows human-parseable duration and not just number of nano-seconds
- Added WithMapContext(...) to the context package. This is a useful function
so I pulled it out of the main.go in cmd/registry so that it can be used
elsewhere.
Docker-DCO-1.1-Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)
through proxies.
Add a function to examine X-Forward-For and X-Real-Ip headers for
originating IP addresses. Use RemoteAddr for notification request
record and HTTP request context.
The new context package supports context-aware logging, integrating with
logrus. Several utilities are provided to associate http requests with a
context, ensuring that one can trace log messages all the way through a
context-aware call stack.
A full description of this functionality is available in doc.go.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
