Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions codegen/protocol-tests/model/error-correction-tests.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ operation SayHelloXml { output: TestOutput, errors: [Error] }

structure TestOutputDocument with [TestStruct] {
innerField: Nested,
// FIXME: This trait fails smithy validator
// @required
@required
document: Document
}
structure TestOutput with [TestStruct] { innerField: Nested }
Expand All @@ -65,8 +64,7 @@ structure TestStruct {
@required
nestedListValue: NestedList

// FIXME: This trait fails smithy validator
// @required
@required
nested: Nested

@required
Expand Down Expand Up @@ -97,8 +95,7 @@ union MyUnion {
}

structure Nested {
// FIXME: This trait fails smithy validator
// @required
@required
a: String
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,7 @@ abstract class AwsHttpBindingProtocolGenerator : HttpBindingProtocolGenerator()
// val targetedTest = TestMemberDelta(setOf("RestJsonComplexErrorWithNoMessage"), TestContainmentMode.RUN_TESTS)

val ignoredTests = TestMemberDelta(
setOf(
"AwsJson10ClientErrorCorrectsWithDefaultValuesWhenServerFailsToSerializeRequiredValues",
"RestJsonNullAndEmptyHeaders",
"NullAndEmptyHeaders",
"RpcV2CborClientPopulatesDefaultsValuesWhenMissingInResponse",
"RpcV2CborClientPopulatesDefaultValuesInInput",
),
setOf(),
)

val requestTestBuilder = HttpProtocolUnitTestRequestGenerator.Builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
.takeUnless { it.isEmpty() }
?.let { "ByteStream.fromString(${it.dq()})" }

targetShape.isBlobShape -> "${node.toString().dq()}.encodeToByteArray()"
targetShape.isBlobShape -> "${node.toString().dq()}.decodeBase64().encodeToByteArray()"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Don't we need this for streaming targets too? (i.e., the when branch above this one)


targetShape.isDocumentShape -> getDefaultValueForDocument(node)
targetShape.isTimestampShape -> getDefaultValueForTimestamp(node.asNumberNode().get())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,11 @@ class StructureGenerator(
} else {
memberSymbol
}

if (builderMemberSymbol.shape is BlobShape && builderMemberSymbol.isNotNullable) {
writer.addImport(RuntimeTypes.Core.Text.Encoding.decodeBase64)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decodeBase64 is used in KotlinSymbolProvider, but we don't have access to the KotlinWriter there, so the import gets added here if necessary. Any other ideas to work around this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, we don't have a KotlinWriter (nor should we) in KotlinSymbolProvider. But we're still providing symbols and symbols can declare additional imports. We should be setting up those references before returning from the symbol provider.

This probably means a bit of a light refactor in the symbol provider since it's mostly slinging text strings around and wrapping it into a symbol right before returning. But we probably need better awareness of non-textual symbol info in places like getDefaultValue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a minimal refactor to give getDefaultValue access to the Symbol being built, calling addReferences where appropriate


write("public var #L: #E", memberName, builderMemberSymbol)
}
write("")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,8 @@ class HttpStringValuesMapSerializer(
val paramName = binding.locationName
// addAll collection parameter 2
val param2 = if (mapFnContents.isEmpty()) "input.$memberName" else "input.$memberName.map { $mapFnContents }"
val nullCheck = if (memberSymbol.isNullable) "?" else ""
writer.write(
"if (input.#L$nullCheck.isNotEmpty() == true) #L(#S, #L)",
"if (input.#L != null) #L(#S, #L)",
memberName,
binding.location.addAllFnName,
paramName,
Expand All @@ -174,8 +173,7 @@ class HttpStringValuesMapSerializer(
val paramName = binding.locationName
val memberSymbol = symbolProvider.toSymbol(binding.member)

// NOTE: query parameters are allowed to be empty, whereas headers should omit empty string
// values from serde
// NOTE: query parameters are allowed to be empty
if ((location == HttpBinding.Location.QUERY || location == HttpBinding.Location.HEADER) && binding.member.hasTrait<IdempotencyTokenTrait>()) {
// Call the idempotency token function if no supplied value.
writer.addImport(RuntimeTypes.SmithyClient.IdempotencyTokenProviderExt)
Expand All @@ -185,18 +183,7 @@ class HttpStringValuesMapSerializer(
paramName,
)
} else {
val nullCheck =
if (location == HttpBinding.Location.QUERY ||
memberTarget.hasTrait<
@Suppress("DEPRECATION")
software.amazon.smithy.model.traits.EnumTrait,
>()
) {
if (memberSymbol.isNullable) "input.$memberName != null" else ""
} else {
val nullCheck = if (memberSymbol.isNullable) "?" else ""
"input.$memberName$nullCheck.isNotEmpty() == true"
}
val nullCheck = if (memberSymbol.isNullable) "input.$memberName != null" else ""

val cond = defaultCheck(binding.member) ?: nullCheck

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class SymbolProviderTest {
"double,2.71828,2.71828",
"byte,10,10.toByte()",
"string,\"hello\",\"hello\"",
"blob,\"abcdefg\",\"abcdefg\".encodeToByteArray()",
"blob,\"abcdefg\",\"abcdefg\".decodeBase64().encodeToByteArray()",
"boolean,true,true",
"bigInteger,5,5",
"bigDecimal,9.0123456789,9.0123456789",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ internal class SmokeTestOperationSerializer: HttpSerializer.NonStreaming<SmokeTe
}

builder.headers {
if (input.header1?.isNotEmpty() == true) append("X-Header1", input.header1)
if (input.header2?.isNotEmpty() == true) append("X-Header2", input.header2)
if (input.header1 != null) append("X-Header1", input.header1)
if (input.header2 != null) append("X-Header2", input.header2)
}

val payload = serializeSmokeTestOperationBody(context, input)
Expand Down Expand Up @@ -264,7 +264,7 @@ internal class TimestampInputOperationSerializer: HttpSerializer.NonStreaming<Ti
}
parameters.decodedParameters(PercentEncoding.SmithyLabel) {
if (input.queryTimestamp != null) add("qtime", input.queryTimestamp.format(TimestampFormat.ISO_8601))
if (input.queryTimestampList?.isNotEmpty() == true) addAll("qtimeList", input.queryTimestampList.map { it.format(TimestampFormat.ISO_8601) })
if (input.queryTimestampList != null) addAll("qtimeList", input.queryTimestampList.map { it.format(TimestampFormat.ISO_8601) })
}
}

Expand Down Expand Up @@ -304,7 +304,7 @@ internal class BlobInputOperationSerializer: HttpSerializer.NonStreaming<BlobInp
}

builder.headers {
if (input.headerMediaType?.isNotEmpty() == true) append("X-Blob", input.headerMediaType.encodeBase64())
if (input.headerMediaType != null) append("X-Blob", input.headerMediaType.encodeBase64())
}

val payload = serializeBlobInputOperationBody(context, input)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ class HttpStringValuesMapSerializerTest {
contents.assertBalancedBracesAndParens()

val expectedContents = """
if (input.header1?.isNotEmpty() == true) append("X-Header1", input.header1)
if (input.header2?.isNotEmpty() == true) append("X-Header2", input.header2)
if (input.header1 != null) append("X-Header1", input.header1)
if (input.header2 != null) append("X-Header2", input.header2)
""".trimIndent()
contents.shouldContainOnlyOnceWithDiff(expectedContents)
}
Expand All @@ -157,7 +157,7 @@ class HttpStringValuesMapSerializerTest {
contents.assertBalancedBracesAndParens()

val expectedContents = """
if (input.headerMediaType?.isNotEmpty() == true) append("X-Blob", input.headerMediaType.encodeBase64())
if (input.headerMediaType != null) append("X-Blob", input.headerMediaType.encodeBase64())
""".trimIndent()
contents.shouldContainOnlyOnceWithDiff(expectedContents)
}
Expand All @@ -168,10 +168,10 @@ class HttpStringValuesMapSerializerTest {
contents.assertBalancedBracesAndParens()

val expectedContents = """
if (input.enumList?.isNotEmpty() == true) appendAll("x-enumList", input.enumList.map { quoteHeaderValue(it.value) })
if (input.intList?.isNotEmpty() == true) appendAll("x-intList", input.intList.map { it.toString() })
if (input.strList?.isNotEmpty() == true) appendAll("x-strList", input.strList.map { quoteHeaderValue(it) })
if (input.tsList?.isNotEmpty() == true) appendAll("x-tsList", input.tsList.map { it.format(TimestampFormat.RFC_5322) })
if (input.enumList != null) appendAll("x-enumList", input.enumList.map { quoteHeaderValue(it.value) })
if (input.intList != null) appendAll("x-intList", input.intList.map { it.toString() })
if (input.strList != null) appendAll("x-strList", input.strList.map { quoteHeaderValue(it) })
if (input.tsList != null) appendAll("x-tsList", input.tsList.map { it.format(TimestampFormat.RFC_5322) })
""".trimIndent()
contents.shouldContainOnlyOnceWithDiff(expectedContents)
}
Expand All @@ -190,7 +190,7 @@ class HttpStringValuesMapSerializerTest {
val queryContents = getTestContents(defaultModel, "com.test#TimestampInput", HttpBinding.Location.QUERY)
val expectedQueryContents = """
if (input.queryTimestamp != null) add("qtime", input.queryTimestamp.format(TimestampFormat.ISO_8601))
if (input.queryTimestampList?.isNotEmpty() == true) addAll("qtimeList", input.queryTimestampList.map { it.format(TimestampFormat.ISO_8601) })
if (input.queryTimestampList != null) addAll("qtimeList", input.queryTimestampList.map { it.format(TimestampFormat.ISO_8601) })
""".trimIndent()
queryContents.shouldContainOnlyOnceWithDiff(expectedQueryContents)
}
Expand Down
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ micrometer-version = "1.14.2"
binary-compatibility-validator-version = "0.16.3"

# codegen
smithy-version = "1.53.0"
smithy-version = "1.54.0"
smithy-gradle-version = "0.9.0"

# testing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,17 @@ public fun String.decodeBase64Bytes(): ByteArray = encodeToByteArray().decodeBas
* Decode [ByteArray] from base64 format
*/
public fun ByteArray.decodeBase64(): ByteArray {
val encoded = this
// Calculate the padding needed to make the length a multiple of 4
val remainder = size % 4
val encoded: ByteArray = if (remainder == 0) {
this
} else {
this + ByteArray(4 - remainder) { BASE64_PAD.code.toByte() }
}

val decodedLen = base64DecodedLen(encoded)
val decoded = ByteArray(decodedLen)
val blockCnt = size / 4
val blockCnt = encoded.size / 4
var readIdx = 0
var writeIdx = 0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,6 @@ class Base64Test {
ex.message!!.shouldContain("decode base64: invalid input byte: 45")
}

@Test
fun decodeNonMultipleOf4() {
val ex = assertFails {
"Zm9vY=".decodeBase64()
}
ex.message!!.shouldContain("invalid base64 string of length 6; not a multiple of 4")
}

@Test
fun decodeInvalidPadding() {
val ex = assertFails {
Expand Down Expand Up @@ -116,4 +108,24 @@ class Base64Test {
assertEquals(encoded, decoded.encodeBase64())
assertEquals(decoded, encoded.decodeBase64())
}

@Test
fun testUnpaddedInputs() {
// from https://github.com/smithy-lang/smithy/pull/2502
val input = "v2hkZWZhdWx0c79tZGVmYXVsdFN0cmluZ2JoaW5kZWZhdWx0Qm9vbGVhbvVrZGVmYXVsdExpc3Sf/3BkZWZhdWx0VGltZXN0YW1wwQBrZGVmYXVsdEJsb2JDYWJja2RlZmF1bHRCeXRlAWxkZWZhdWx0U2hvcnQBbmRlZmF1bHRJbnRlZ2VyCmtkZWZhdWx0TG9uZxhkbGRlZmF1bHRGbG9hdPo/gAAAbWRlZmF1bHREb3VibGX6P4AAAGpkZWZhdWx0TWFwv/9rZGVmYXVsdEVudW1jRk9PbmRlZmF1bHRJbnRFbnVtAWtlbXB0eVN0cmluZ2BsZmFsc2VCb29sZWFu9GllbXB0eUJsb2JAaHplcm9CeXRlAGl6ZXJvU2hvcnQAa3plcm9JbnRlZ2VyAGh6ZXJvTG9uZwBpemVyb0Zsb2F0+gAAAABqemVyb0RvdWJsZfoAAAAA//8"
input.decodeBase64()

val inputs = mapOf<String, String>(
"YQ" to "a",
"Yg" to "b",
"YWI" to "ab",
"YWJj" to "abc",
"SGVsbG8gd29ybGQ" to "Hello world",
)

inputs.forEach { (encoded, expected) ->
val actual = encoded.decodeBase64()
assertEquals(expected, actual)
}
}
}
Loading