Comments (13)
@ncw Expect: 100-continue can help with this, since no data will be sent in case token had expired.
from swift.
I've merged a fix for this in 675ef0f
I've tested against all the swift servers I have access to and also run the rclone integration tests with it and all appears to be well.
I'd appreciate any test reports before I tag the release - thanks
from swift.
@databus23 That's your problem with the version resource.
from swift.
I see what you mean...
It shouldn't be attempting retries on 401 if p.Body != nil
.
Fancy making a PR?
from swift.
But failing with 401 on all requests with p.Body != nil
kind of disrupts the normal flow of using this client where the re-authentication is handled transparently by the client transparently.
This would mean the user needs to implement reauth for write operations.
from swift.
@databus23 the client would handle the re-auth on the next call to the api.
Short of buffering a potentially very large input buffer (remember you can't re-wind an io.Reader
), I can't see a way around this - we have to fail the operation and the caller has to restart it.
What we are doing at the moment is just wrong - if a reauth happens while we are uploading a file, we'll upload a 0 sized file instead.
Looking at the code Body:
is mostly io.Reader
s passed in by the client. However there are a few which are bytes.Buffer
. These could be retried potentially.
Also we could attempt to re-wind the io.Reader
by attempting to see if it supports the io.Seeker
interface. We'd have to make sure we read the start position though. If we had a Seekable
bytes.Buffer
that would help.
My preferred option is to return a specific error swift.RetryNeeded
and leave this to the client though.
from swift.
@gschukin Interesting idea. A bit of research indicates that Expect: 100-continue
is supported in go >= 1.6.
So the optimal solution might be for go < 1.6 return the swift.RetryNeeded
error and for go >= 1.6 add a Expect: 100-continue
header (if p.Body
is not nil).
I'm not clear on whether all swift instances support 100 continue though? A read of the HTTP/1.1 spec indicates that 100-continue support is mandatory for servers. But does it work through everyone's proxies etc?
One could belt-and braces this by wrapping the p.Body
to discover whether it actually got read from or not.
from swift.
@ncw from what I find out, Swift 1.7.5 already supports Expect: 100-continue
, it was 4 years ago.
Also I assume that all proxies that support HTTP/1.1 must support Expect: 100-continue
From RFC
- If a proxy receives a request that includes an Expect request-
header field with the "100-continue" expectation, and the proxy
either knows that the next-hop server complies with HTTP/1.1 or
higher, or does not know the HTTP version of the next-hop
server, it MUST forward the request, including the Expect header
field.
- If the proxy knows that the version of the next-hop server is
HTTP/1.0 or lower, it MUST NOT forward the request, and it MUST
respond with a 417 (Expectation Failed) status.
- Proxies SHOULD maintain a cache recording the HTTP version
numbers received from recently-referenced next-hop servers.
- A proxy MUST NOT forward a 100 (Continue) response if the
request message was received from an HTTP/1.0 (or earlier)
client and did not include an Expect request-header field with
the "100-continue" expectation. This requirement overrides the
general rule for forwarding of 1xx responses (see section 10.1).
from swift.
I've been doing some more reading on expect 100-continue... There do appear to be load balancers which don't support it (eg Google's). An HTTP/1.1 conformant implementation should reply with a 417 error if it can't support the expectation which would allow us to turn off expect 100-continue support.
That probably needs to be a config option though, probably defaulting to use expect 100-continue, so probably DisableExpect100Continue
or something like that. If a 417 error is received then we can set that flag also and have it set for go < 1.6.
from swift.
That turned out to be a lot more complicated than I'd hoped :-(
I made a branch here if anyone would like to take a look I'd be grateful: https://github.com/ncw/swift/compare/expect-continue
I've tested this by setting the integration tests to offer up an invalid token every transaction to force a 401 error and a token refresh and all the tests work fine like this except for BulkUpload
which I really don't think I can fix as the error return isn't reported over http.
Research into getting this to work also caused me to report 2 issues on go - the first one being the most surprising that net/http read 1 byte from my reader even on 401 errors.
from swift.
@ncw I'll talk to my vendor as I assume that 401 should not be returned within responce body
from swift.
Hello! I've made a PR which should hopefully fix this :)
from swift.
@ncw This issue was fixed? waiting for merge into master?
from swift.
Related Issues (20)
- Add application credential auth support
- Support for symlink
- Support status 429 for OVH Cloud Archive HOT 1
- Some questions about StorageUrl and Mutex in Connection struct HOT 4
- Bulk upload method returns err if content type exists charset HOT 2
- Typo ? in DynamicLargeObjectMove HOT 1
- Header is not processed in DynamicLargeObjectCreate HOT 2
- OS_CACERT env variable is not used HOT 3
- Default user domain in password Auth v3 & discarded server error message in Auth HOT 1
- Add inspectable context parameter to API calls HOT 7
- Panic calling DynamicLargeObjectCreateFile with unauthenticated connection HOT 3
- Add support for passing context to API calls HOT 2
- Migrate from travis.org HOT 1
- Time difference between server and client HOT 9
- ObjectCreate() 404 return is wrongly interpreted as ObjectNotFound instead of ContainerNotFound
- slo: no headers for segments HOT 1
- Document about authentication flow may be misleading?
- No support for rate-limit errors with retry-after headers.
- Support for parsing json in ObjectNames()
- Documentation throws 404... HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from swift.