cnabio / cnab-go

A Go implementation of CNAB Core 1.0
MIT License
69 stars 37 forks source link

Decide how to marshall bundles #106

Open glyn opened 5 years ago

glyn commented 5 years ago

The current bundle marshalling code in cnab-go does not add a newline unlike the bundle marshalling code in duffle build:

func marshalBundle(bf *bundle.Bundle) ([]byte, string, error) {
    data, err := json.MarshalCanonical(bf)
        ...
    data = append(data, '\n') //TODO: why?

    digest, err := digest.OfBuffer(data)
        ...
    return data, digest, nil
}

Note: the newline is added before the digest is calculated.

The newline seems to have originated in some earlier code which signed the data first and then added the newline before the bundle was written to file, thus:

func (b *buildCmd) writeBundle(loc string, bf *bundle.Bundle) error {
        ...
    data, err := sign.Clearsign(bf)
    data = append(data, '\n')
    ...
    return ioutil.WriteFile(loc, data, 0644)
}

Now that we are planning to move bundle marshalling to cnab-go, we need to decide whether to add the newline or not.

technosophos commented 5 years ago

This was to make it compatible with other tools that signed things. Some of them (I think it was GnuPG) automatically add a newline before signing.

glyn commented 5 years ago

Thanks! So, given that we don't implement signing at the moment, I'd vote for not adding the newline. (We can revisit this when signing is added back.)

technosophos commented 5 years ago

Is there any case in which removing now and adding it later will break backward compatibility?

glyn commented 5 years ago

Possibly, if we added it back in later without that being part of signing, but that would seem to be unnecessary. However, if we add it back only when re-implementing signing, then backward compatibility is unlikely to be an issue because signed and unsigned bundles do not need to interchange with each other. If we strip off the signature from a signed bundle to convert it to unsigned, we can presumably strip off the newline too.

If on the other hand we keep the newline, then not only will we be stuck with it, but I think we'll need to add another newline if we re-implement signing, for the reason you gave earlier.

So I'd still vote for not adding the newline. What do you think @technosophos?

technosophos commented 5 years ago

OK. That's fine