Skip to content

Comments

fix Date header#1024

Merged
sylvestre merged 2 commits intomozilla:masterfrom
fluidex:bugs/date-header
Nov 3, 2021
Merged

fix Date header#1024
sylvestre merged 2 commits intomozilla:masterfrom
fluidex:bugs/date-header

Conversation

@lightsing
Copy link
Contributor

@lightsing lightsing commented Jul 29, 2021

The Date header in s3 module doesn't obey the RFC7231 definition.
Causing s3-compatible server refuses sccache's request.

original request:

GET /0/1/2/012620f0a29db1fa159c8471bb82f9c25cad332cfffa5f680f7bb0cfd9c30e62 HTTP/1.1
date: Thu, 29 Jul 2021 06:56:44 +0000
authorization: AWS **RETRACTED**
user-agent: reqwest/0.9.24
accept: */*
accept-encoding: gzip
host: **RETRACTED**.oss-accelerate.aliyuncs.com

server response:

HTTP/1.1 403 Forbidden
Server: AliyunOSS
Date: Thu, 29 Jul 2021 06:56:44 GMT
Content-Type: application/xml
Content-Length: 252
Connection: keep-alive
x-amz-request-id: **RETRACTED**
x-oss-server-time: 0

<?xml version="1.0" encoding="UTF-8"?>
<Error>
  <Code>AccessDenied</Code>
  <Message>OSS authentication requires a valid Date.</Message>
  <RequestId>6102512CBB04C5A3079D59B5</RequestId>
  <HostId>sccache.oss-accelerate.aliyuncs.com</HostId>
</Error>

patched request:

GET /c/c/6/cc65e6de3e04ef4f9eaae65e71fbba04019c78792364467217248f1b06bc3e71 HTTP/1.1
date: Thu, 29 Jul 2021 07:28:02 GMT
authorization: AWS **RETRACTED**
user-agent: reqwest/0.9.24
accept: */*
accept-encoding: gzip
host: **RETRACTED**.oss-accelerate.aliyuncs.com

@sylvestre
Copy link
Collaborator

sylvestre commented Oct 26, 2021

Could you please add a unit test for this? thanks

@sylvestre
Copy link
Collaborator

sylvestre commented Nov 2, 2021

I was hoping for a unit test for the feature or the sccache function, not the date function itself :)

@lightsing
Copy link
Contributor Author

lightsing commented Nov 2, 2021

The s3::Bucket::get doesn't have unit test. It would be way more complex for mocking s3 request. And it doesn't make sense to add unit test on s3::Bucket::get for this change.

@sylvestre sylvestre merged commit 7b25245 into mozilla:master Nov 3, 2021
@Xanewok Xanewok mentioned this pull request Nov 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

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

@lightsing @sylvestre