bc-csharp

bc-csharp copied to clipboard

PGP encrypted armored files always contain the "Version" header, also after setting the header value to null (-> "Version: "). Could you please change this, so that when setting the version header to null it's completely removed?

Proposed change (https://github.com/bcgit/bc-csharp/blob/master/crypto/src/bcpg/ArmoredOutputStream.cs):

Current implementation

public ArmoredOutputStream(Stream outStream, IDictionary<string, string> headers)
            : this(outStream)
        {
            foreach (var header in headers)
            {
                var headerList = new List<string>(1);
                headerList.Add(header.Value);
                m_headers[header.Key] = headerList;
            }
        }

Solution

public ArmoredOutputStream(Stream outStream, IDictionary<string, string> headers)
        : this(outStream)
    {
        foreach (var header in headers)
            SetHeader(header.Key, header.Value);
    }

@boegi1 Sorry if I'm missing something, but could call headers.Remove("Version") instead of setting it to null?

@cipherboy My thoughts on this were that the SetHeader(header.Key, header.Value) function already contains the logic to remove a header where the value for a key is null. I also do not see any way to remove headers from the ArmoredOutputStream other than explicitly calling the SetHeader("Version", null) function. So wouldn't implicitly using it simplify it? Please correct me if I miss something.

@boegi1 Right, I think this is fine for after-the-fact removal. But I'm curious about why you'd want to pass a dictionary with a header key (set to the null value) to the constructor in the first place, versus not including the header in the constructor's header dictionary at all, given you don't want them in the AOS? :-) My 2c.

@cipherboy Doesn't passing an empty header Dictionary automatically add the version header?

Ah, so we do:

using Org.BouncyCastle.Bcpg;

var headers = new Dictionary<string, string>();
ArmoredOutputStream aos = new ArmoredOutputStream(Console.OpenStandardOutput(), headers);

aos.WriteByte(0x00);
aos.Close();

I think this change makes sense, but I'll double check with @peterdettman that he doesn't think it'll break anything. Thanks!

I chose to add constructors with an addVersionHeader argument instead.