Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

COPY instructions shouldn't be allowed to cross the source segment/target window boundary #85

Open
mverver-google opened this issue Jun 29, 2019 · 0 comments

Comments

@mverver-google
Copy link

In the encoder source code, it is claimed that it's possible for a COPY instruction to begin in the source segment and extend past its end, into the target window.
https://github.com/google/open-vcdiff/blob/master/src/google/vcencoder.h#L97-L102

And the decoder supports such instructions:
https://github.com/google/open-vcdiff/blob/master/src/vcdecoder.cc#L1209-L1216

However, this contradicts the specification in RFC 3284, Section 3:

A target window T, of length t, may be compared against some source
data segment S, of length s. [..]

Assume that S[j] represents the jth byte in S, and T[k] represents
the kth byte in T. Then, for the delta instructions, we treat the
data windows S and T as substrings of a superstring U, formed by
concatenating them like this:

  S[0]S[1]...S[s-1]T[0]T[1]...T[t-1]

[..]
COPY: This instruction has two arguments, a size x and an address
p in the string U. The arguments specify the substring of U
that must be copied. We shall assert that such a substring
must be entirely contained in either S or T.

(Emphasis mine.) This last sentence seems intended to specifically forbid COPY instructions which cross the source segment/target window boundary.

The open-vcdiff implementation is still compliant because the encoder doesn't generate such instructions, but to avoid confusion about the file format, it might be helpful to:

  • In the encoder: remove the comment.
  • In the decoder: either reject inputs that contain these invalid instructions, or at least add a comment mentioning that such instructions are non-standard.
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

No branches or pull requests

1 participant