Add support for CRLF line endings to PEMDocument
#68
Conversation
There was a problem hiding this comment.
Thanks for this! However, I think we should consider taking a different tack.
Rather than ask the user which line-ending is in use when we parse, we should detect it ourselves. As we know what the start of the PEM document looks like, we can work out what the line ending format will be. Bonus points for us if we can tolerate both line endings mixed arbitrarily, but worst-case we should look for the first line ending and work it out from there.
As a sidebar, I don't think we need support for serializing in both formats: we can confidently just serialize with LF line ending only.
| @@ -218,6 +218,25 @@ struct LazyPEMDocument { | |||
| } | |||
|
|
|||
| extension Substring.UTF8View { | |||
| /// Checks whether `self` starts with a new line character. | |||
| /// - Returns: `true` if `self` starts with a new line character; otherwise, `false`. | |||
| fileprivate func startsWithNewLine() -> Bool { | |||
There was a problem hiding this comment.
| fileprivate func startsWithNewLine() -> Bool { | |
| fileprivate var startsWithNewLine(): Bool { |
There was a problem hiding this comment.
Removed startsWithNewLine entirely since there are no longer any references to it
| ) | ||
| suffix: "-----" | ||
| ), | ||
| self[beginDiscriminatorSuffix.upperBound...].startsWithNewLine(), |
There was a problem hiding this comment.
This line is redundant: the guard let below covers it.
There was a problem hiding this comment.
Agreed. Removed this and just keeping the call to offsetNewLine in the line below.
| // The current line cannot contain any "\n" and the end index must be a new line. | ||
| guard message[..<expectedNewLineIndex].firstIndex(of: UInt8(ascii: "\n")) == nil, | ||
| message[expectedNewLineIndex...].startsWithNewLine(), | ||
| let nextLineStart = message[expectedNewLineIndex...].offsetNewLine() |
There was a problem hiding this comment.
This guard let also supersedes the other call.
There was a problem hiding this comment.
Agreed, like above.
|
Nice patch, thanks! ✨ |
Motivation:
As per RFC 1421, an encoded PEM message is divided into text lines of 64 characters each:
We currently only support parsing PEM strings with
LFline endings. We would like to add support for PEM strings containingCRLFline endings too (GitHub Issue #66).Modifications:
Added an additional argument
lineEndingtoPEMDocument's initializers and functions. This argument defaults toLF.Result:
PEM strings containing
CRLFline endings can be used to construct aPEMDocument.