Skip to content

Fix size validation bug with some registries#2364

Merged
estesp merged 1 commit intocontainerd:masterfrom
dmcgowan:fix-http-seeker-unsupported-range
May 29, 2018
Merged

Fix size validation bug with some registries#2364
estesp merged 1 commit intocontainerd:masterfrom
dmcgowan:fix-http-seeker-unsupported-range

Conversation

@dmcgowan
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan commented May 27, 2018

Ensures that the client can handle cases where the registry ignores content range.

Some registries ignore the content range and in some cases supports inconsistently. For example the Docker Hub appears to use Cloudflare and Cloudfront for serving content, cloudflare will ignore the content range while cloudfront uses it. This has lead to many cases of the mismatched layer size error on pull but without a consistent reproduction.

Closes #2306

Ensures that the client can handle cases where the
registry ignores content length.

Signed-off-by: Derek McGowan <[email protected]>
@dmcgowan dmcgowan changed the title Fix invalid length bug with some registries Fix size validation bug with some registries May 27, 2018
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #2364 into master will increase coverage by 0.11%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2364      +/-   ##
==========================================
+ Coverage   44.99%   45.11%   +0.11%     
==========================================
  Files          92       92              
  Lines        9336     9358      +22     
==========================================
+ Hits         4201     4222      +21     
  Misses       4457     4457              
- Partials      678      679       +1
Flag Coverage Δ
#linux 49.34% <63.63%> (+0.04%) ⬆️
#windows 41.4% <75%> (+0.15%) ⬆️
Impacted Files Coverage Δ
remotes/docker/fetcher.go 54.34% <75%> (+12.91%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cecf576...59740d8. Read the comment docs.

@AkihiroSuda
Copy link
Copy Markdown
Member

Is 1.0.x affected?

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 7f800e0 into containerd:master May 29, 2018
@dmcgowan
Copy link
Copy Markdown
Member Author

@AkihiroSuda it is client side, so the daemon is not affected, but the clients are. I can backport to 1.0.x for those vendoring the client from 1.0. My main concern is 1.1 since the cri plugin does use the client code for pulling. I'll open a PR to both though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants