Skip to content

Commit

Permalink
Limit length of package ids to 64 characters (#10368)
Browse files Browse the repository at this point in the history
* Limit length of package ids to 64 characters

In practice, you cannot use a package id that is not a valid sha256
outside of a test anyway since we validate them. However, we didn’t
enforce it at the daml-lf level which is a bit annoying.

This PR fixes this and adds a length restriction of 64
characters (i.e. 256 bytes for sha256).

We could go a bit further and also restrict the characters to valid
sha256 hashes (i.e. 64 character hex strings). I don’t feel all that
strongly about that either way. We use other characters in tests but
fixing that shouldn’t be particularly hard either. The extra
characters don’t seem to cause problem so far, so I think it’s fine to
keep that.

No changelog entry since I don’t see how a user can be affected by this.

changelog_begin
changelog_end

* Update daml-lf/encoder/src/main/scala/com/digitalasset/daml/lf/archive/testing/DamlLfEncoder.scala
  • Loading branch information
cocreature authored Jul 22, 2021
1 parent a56cfea commit 66284c1
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ private[data] final class IdStringImpl extends IdString {
*/
override type PackageId = String
override val PackageId: ConcatenableStringModule[PackageId, HexString] =
new ConcatenableMatchingStringModule("Daml-LF Package ID", "-_ ")
new ConcatenableMatchingStringModule("Daml-LF Package ID", "-_ ", 64)

/** Used to reference to leger objects like contractIds, ledgerIds,
* transactionId, ... We use the same type for those ids, because we
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ class RefTest extends AnyFreeSpec with Matchers with EitherValues {
"reject too long string" in {
Party.fromString("p" * 255) shouldBe a[Right[_, _]]
Party.fromString("p" * 256) shouldBe a[Left[_, _]]
PackageId.fromString("p" * 64) shouldBe a[Right[_, _]]
PackageId.fromString("p" * 65) shouldBe a[Left[_, _]]
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ExceptionTest extends AnyWordSpec with Matchers with TableDrivenPropertyCh

implicit val defaultParserParameters: ParserParameters[this.type] = {
ParserParameters(
defaultPackageId = Ref.PackageId.assertFromString("-pkgId-"),
defaultPackageId = Ref.PackageId.assertFromString("pkgId"),
languageVersion = LanguageVersion.v1_dev,
)
}
Expand Down
11 changes: 2 additions & 9 deletions daml-lf/spec/daml-lf-1.rst
Original file line number Diff line number Diff line change
Expand Up @@ -375,17 +375,10 @@ instances when we want to avoid empty identifiers, escaping problems,
and other similar pitfalls. ::

PackageId strings
PackageIdString ::= ' PackageIdChars ' -- PackageIdString

Sequences of PackageId character
PackageIdChars ::= PackageIdChar -- PackageIdChars
| PackageIdChars PackageIdChar

PackageId character
PackageIdChar ∈ [a-zA-Z0-9\-_ ] -- PackageIdChar
PackageIdString ::= [a-zA-Z0-9\-_ ]{1,64} -- PackageIdString

PartyId strings
PartyIdString ∈ [a-zA-Z0-9:\-_ ]{1,255} -- PartyIdChar
PartyIdString ∈ [a-zA-Z0-9:\-_ ]{1,255} -- PartyIdString

PackageName strings
PackageNameString ∈ [a-zA-Z0-9:\-_]+ -- PackageNameString
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ object ValueGenerators {

// generate a junk identifier
val idGen: Gen[Identifier] = for {
n <- Gen.choose(1, 200)
n <- Gen.choose(1, 64)
packageId <- Gen
.listOfN(n, Gen.alphaNumChar)
.map(s => PackageId.assertFromString(s.mkString))
Expand Down

0 comments on commit 66284c1

Please sign in to comment.