Feature/ktxio #164

JesusMcCloud · 2024-10-21T14:33:47Z

use Kotlinx-io for decoding to avoid unneeded byte copying

iaik-jheher

partial review pending partial rewrite

iaik-jheher · 2024-10-21T14:44:24Z

indispensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/asn1/Asn1Elements.kt

+        (children?.fold(Buffer()) { acc, extendedTlv -> acc.apply { write(extendedTlv.derEncoded) } }
+            ?.let {
+                Buffer().apply { write(tlv.tag.encodedTag); encodeLength(it.size); it.transferTo(this) }
+            }?.readByteArray()
+            ?: Buffer().apply { write(tlv.tag.encodedTag); write(encodedLength);write(tlv.content) }.readByteArray())


i suggest an if expression (or improved indentation)

iaik-jheher · 2024-10-21T14:46:28Z

indispensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/asn1/Asn1Elements.kt

-        children?.fold(byteArrayOf()) { acc, extendedTlv -> acc + extendedTlv.derEncoded }
-            ?.let { byteArrayOf(*tlv.tag.encodedTag, *it.size.encodeLength(), *it) }
-            ?: byteArrayOf(*tlv.tag.encodedTag, *encodedLength, *tlv.content)
+        (children?.fold(Buffer()) { acc, extendedTlv -> acc.apply { write(extendedTlv.derEncoded) } }


consider Buffer.writeDerEncoded or similar to avoid copying to bytearray at every layer of the hierarchy

iaik-jheher · 2024-10-21T14:49:34Z

indispensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/asn1/Asn1Elements.kt

@@ -120,11 +122,9 @@ sealed class Asn1Element(
    /**
     * Convenience method to directly produce an HEX string of this element's ANS.1 representation


Suggested change

* Convenience method to directly produce an HEX string of this element's ANS.1 representation

* Convenience method to directly produce an HEX string of this element's ASN.1 representation

iaik-jheher · 2024-10-21T14:49:43Z

indispensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/asn1/Asn1Elements.kt

+    fun toDerHexString(lineLen: Byte? = null) = @OptIn(ExperimentalStdlibApi::class)
+    derEncoded.toHexString(HexFormat.UpperCase)
+        .let { if (lineLen == null) it else it.chunked(lineLen.toInt()).joinToString("\n") }


broken indentation

iaik-jheher · 2024-10-21T14:50:07Z

indispensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/asn1/Asn1Elements.kt

@@ -240,8 +240,12 @@ sealed class Asn1Element(
        @Serializable(with = ByteArrayBase64Serializer::class) val encodedTag: ByteArray
    ) : Comparable<Tag> {
        private constructor(values: Triple<ULong, Int, ByteArray>) : this(values.first, values.second, values.third)
+
+        //TODO this CTOR is internally called only using already validated inputs.


outstanding todo

iaik-jheher · 2024-10-21T14:53:49Z

indispensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/asn1/Asn1Elements.kt

@@ -312,7 +317,7 @@ sealed class Asn1Element(

        override fun toString(): String =
            "${tagClass.let { if (it == TagClass.UNIVERSAL) "" else it.name + " " }}${tagValue}${if (isConstructed) " CONSTRUCTED" else ""}" +
-                    (" (=${encodedTag.encodeToString(Base16)})")
+                    (" (=${@OptIn(ExperimentalStdlibApi::class) encodedTag.toHexString(HexFormat.UpperCase)})")


can we just get a @file:OptIn for this marker

iaik-jheher · 2024-10-21T14:54:33Z

indispensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/asn1/Asn1Elements.kt

@@ -578,8 +583,7 @@ class Asn1CustomStructure private constructor(


    override val content: ByteArray by lazy {
-        if (!tag.isConstructed)
-            children.fold(byteArrayOf()) { acc, asn1Element -> acc + asn1Element.derEncoded }
+        if (!tag.isConstructed) Buffer().apply { children.forEach { write(it.derEncoded) } }.readByteArray()


this should also avoid constant bytearray punning

iaik-jheher · 2024-10-21T14:56:45Z

indispensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/asn1/TLV.kt

@@ -16,11 +19,13 @@ internal data class TLV(val tag: Asn1Element.Tag, val content: ByteArray) {
    val encodedTag get() = tag.encodedTag

    override fun equals(other: Any?): Boolean {
+        if (other is TLV.Shallow || this is TLV.Shallow) throw IllegalStateException("Shallow TLVs may neve be compared")


Suggested change

if (other is TLV.Shallow || this is TLV.Shallow) throw IllegalStateException("Shallow TLVs may neve be compared")

if (other is TLV.Shallow || this is TLV.Shallow) throw IllegalStateException("Shallow TLVs may never be compared")

iaik-jheher · 2024-10-21T15:00:58Z

...ensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/asn1/encoding/Asn1Decoding.kt

 * @see parse
 */
 @Throws(Asn1Exception::class)
 fun Asn1Element.Companion.parseFirst(source: ByteArray): Pair<Asn1Element, ByteArray> =
-    source.iterator().doParseSingle().let { Pair(it, source.copyOfRange(it.overallLength, source.size)) }
+    Buffer().apply { write(source) }.let { it.readAsn1Element() to it.snapshot().toByteArray() }


how is Buffer().apply { write(source) } (used here) different from source.copyToSource() (used elsewhere)?

iaik-jheher · 2024-10-21T15:02:12Z

...ensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/asn1/encoding/Asn1Decoding.kt

+    if (tlv.isSequence()) Asn1Sequence(tlv.content.doParseAll())
+    else if (tlv.isSet()) Asn1Set.fromPresorted(tlv.content.doParseAll())
+    else if (tlv.isExplicitlyTagged())
+        Asn1ExplicitlyTagged(tlv.tag.tagValue, tlv.content.doParseAll())
+    else if (tlv.tag == Asn1Element.Tag.OCTET_STRING) catching {
+        val expected = tlv.content.size
+        Asn1EncapsulatingOctetString(tlv.content.peek().doParseAll()).also {
+            if (it.length != expected) throw Asn1StructuralException("The contents contain a valid structure and garbage.")
+        } as Asn1Element
+    }.getOrElse { Asn1PrimitiveOctetString(tlv.content.readByteArray()) as Asn1Element }
+    else if (tlv.tag.isConstructed) { //custom tags, we don't know if it is a SET OF, SET, SEQUENCE, … so we default to sequence semantics
+        Asn1CustomStructure(tlv.content.doParseAll(), tlv.tag.tagValue, tlv.tagClass)
+    } else Asn1Primitive(tlv.tag, tlv.content.readByteArray())


please indent:

if (a) A else if (b) B

right now it's very hard to follow

JesusMcCloud added 2 commits October 17, 2024 12:38

KTXIO-based number encoding

a6bc66f

KTXIO-based ASN.1 decoding

fd85d07

JesusMcCloud requested a review from iaik-jheher October 21, 2024 14:33

update changelog

7552e01

iaik-jheher reviewed Oct 21, 2024

View reviewed changes

iaik-jheher and others added 2 commits October 21, 2024 18:52

wrap in unsafe source

e0fa1ba

improve tag

4784b65

JesusMcCloud force-pushed the feature/ktxio branch from 8e9da88 to 4784b65 Compare October 21, 2024 21:51

repairs

8f6f5a4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/ktxio #164

Feature/ktxio #164

JesusMcCloud commented Oct 21, 2024

iaik-jheher left a comment

iaik-jheher Oct 21, 2024

iaik-jheher Oct 21, 2024

iaik-jheher Oct 21, 2024

iaik-jheher Oct 21, 2024

iaik-jheher Oct 21, 2024

iaik-jheher Oct 21, 2024

iaik-jheher Oct 21, 2024

iaik-jheher Oct 21, 2024

iaik-jheher Oct 21, 2024

iaik-jheher Oct 21, 2024

		@@ -120,11 +122,9 @@ sealed class Asn1Element(
		/**
		* Convenience method to directly produce an HEX string of this element's ANS.1 representation

	if (other is TLV.Shallow \|\| this is TLV.Shallow) throw IllegalStateException("Shallow TLVs may neve be compared")
	if (other is TLV.Shallow \|\| this is TLV.Shallow) throw IllegalStateException("Shallow TLVs may never be compared")

Feature/ktxio #164

Are you sure you want to change the base?

Feature/ktxio #164

Conversation

JesusMcCloud commented Oct 21, 2024

iaik-jheher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment