Skip to content

Commit

Permalink
proxy: Change the imgid to uint64
Browse files Browse the repository at this point in the history
In PR review for a different issue, the question of what happens
if we hit overflow for the imageid serial was hit.  This feels
pretty unlikely; if I did the math right, it'd require opening
an average of 136 images per second to overflow it in a year.
Nevertheless, in practice what we're sending on the wire is just a JSON
number, and if we extend this to the "max safe JSON number" of 2^53,
it'd take 285,616,414 images per second to overflow in a year, going
from implausible to probably impossible.

With a bit more work of course, we could make this a sparse mapping
and reuse freed numbers, but eh.

Signed-off-by: Colin Walters <walters@verbum.org>
  • Loading branch information
cgwalters committed Jun 30, 2023
1 parent f95931d commit 2538b95
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 16 deletions.
18 changes: 5 additions & 13 deletions cmd/skopeo/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ type activePipe struct {
// openImage is an opened image reference
type openImage struct {
// id is an opaque integer handle
id uint32
id uint64
src types.ImageSource
cachedimg types.Image
}
Expand All @@ -169,9 +169,9 @@ type proxyHandler struct {
cache types.BlobInfoCache

// imageSerial is a counter for open images
imageSerial uint32
imageSerial uint64
// images holds our opened images
images map[uint32]*openImage
images map[uint64]*openImage
// activePipes maps from "pipeid" to a pipe + goroutine pair
activePipes map[uint32]*activePipe
}
Expand Down Expand Up @@ -307,14 +307,6 @@ func (h *proxyHandler) CloseImage(args []any) (replyBuf, error) {
return ret, nil
}

func parseImageID(v any) (uint32, error) {
imgidf, ok := v.(float64)
if !ok {
return 0, fmt.Errorf("expecting integer imageid, not %T", v)
}
return uint32(imgidf), nil
}

// parseUint64 validates that a number fits inside a JavaScript safe integer
func parseUint64(v any) (uint64, error) {
f, ok := v.(float64)
Expand All @@ -328,7 +320,7 @@ func parseUint64(v any) (uint64, error) {
}

func (h *proxyHandler) parseImageFromID(v any) (*openImage, error) {
imgid, err := parseImageID(v)
imgid, err := parseUint64(v)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -828,7 +820,7 @@ func (h *proxyHandler) processRequest(readBytes []byte) (rb replyBuf, terminate
func (opts *proxyOptions) run(args []string, stdout io.Writer) error {
handler := &proxyHandler{
opts: opts,
images: make(map[uint32]*openImage),
images: make(map[uint64]*openImage),
activePipes: make(map[uint32]*activePipe),
}
defer handler.close()
Expand Down
6 changes: 3 additions & 3 deletions integration/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func runTestGetManifestAndConfig(p *proxy, img string) error {
if !ok {
return fmt.Errorf("OpenImage return value is %T", v)
}
imgid := uint32(imgidv)
imgid := uint64(imgidv)
if imgid == 0 {
return fmt.Errorf("got zero from expected image")
}
Expand All @@ -254,7 +254,7 @@ func runTestGetManifestAndConfig(p *proxy, img string) error {
if !ok {
return fmt.Errorf("OpenImageOptional return value is %T", v)
}
imgid2 := uint32(imgidv)
imgid2 := uint64(imgidv)
if imgid2 == 0 {
return fmt.Errorf("got zero from expected image")
}
Expand Down Expand Up @@ -325,7 +325,7 @@ func runTestOpenImageOptionalNotFound(p *proxy, img string) error {
if !ok {
return fmt.Errorf("OpenImageOptional return value is %T", v)
}
imgid := uint32(imgidv)
imgid := uint64(imgidv)
if imgid != 0 {
return fmt.Errorf("Unexpected optional image id %v", imgid)
}
Expand Down

0 comments on commit 2538b95

Please sign in to comment.