From 33761aba456990bdb13db24d9b69e3cde488e98b Mon Sep 17 00:00:00 2001 From: Yuya Hirayama Date: Tue, 18 Nov 2025 06:05:41 +0900 Subject: [PATCH] Fix JSONDecoder error message for decimal-to-Int conversion (issue #1604) This commit fixes the regression where JSONDecoder returns incorrect error information when attempting to decode a decimal value (e.g., 123.45) as Int. **Root Cause (from git history investigation):** - Initial implementation (commit 34c45c1, 2023-03-15) had correct error handling - BufferView refactoring (commit 1ab3832, 2023-04-03) accidentally removed the proper DecodingError.dataCorrupted throw statements - This caused JSONError.numberIsNotRepresentableInSwift to be caught at the top level and converted to a generic error with empty codingPath **Changes:** - Restored proper error handling in `_slowpath_unwrapFixedWidthInteger` - Changed JSONError.numberIsNotRepresentableInSwift throws to DecodingError.dataCorrupted with correct codingPath and debugDescription - Added regression test to prevent future breakage **Before (broken):** - codingPath: [] (empty) - debugDescription: "The given data was not valid JSON." **After (fixed):** - codingPath: ["foo"] - debugDescription: "Parsed JSON number <123.45> does not fit in Int." Fixes #1604 Related: #274 --- .../JSON/JSONDecoder.swift | 20 ++- .../JSONEncoderTests.swift | 116 ++++++++++++++++++ 2 files changed, 134 insertions(+), 2 deletions(-) diff --git a/Sources/FoundationEssentials/JSON/JSONDecoder.swift b/Sources/FoundationEssentials/JSON/JSONDecoder.swift index 53e0032e0..22cdf7d4a 100644 --- a/Sources/FoundationEssentials/JSON/JSONDecoder.swift +++ b/Sources/FoundationEssentials/JSON/JSONDecoder.swift @@ -1000,13 +1000,29 @@ extension JSONDecoderImpl: Decoder { } static private func _slowpath_unwrapFixedWidthInteger(as type: T.Type, json5: Bool, numberBuffer: BufferView, fullSource: BufferView, digitBeginning: BufferViewIndex, for codingPathNode: _CodingPathNode, _ additionalKey: (some CodingKey)?) throws -> T { + // Helper function to create number conversion error + func createNumberConversionError() -> DecodingError { + #if FOUNDATION_FRAMEWORK + let underlyingError: Error? = JSONError.numberIsNotRepresentableInSwift( + parsed: String(decoding: numberBuffer, as: UTF8.self) + ).nsError + #else + let underlyingError: Error? = nil + #endif + return DecodingError.dataCorrupted(DecodingError.Context( + codingPath: codingPathNode.path(byAppending: additionalKey), + debugDescription: "Parsed JSON number <\(String(decoding: numberBuffer, as: UTF8.self))> does not fit in \(T.self).", + underlyingError: underlyingError + )) + } + // This is the slow path... If the fast path has failed. For example for "34.0" as an integer, we try to parse as either a Decimal or a Double and then convert back, losslessly. if let double = Double(prevalidatedBuffer: numberBuffer) { // T.init(exactly:) guards against non-integer Double(s), but the parser may // have already transformed the non-integer "1.0000000000000001" into 1, etc. // Proper lossless behavior should be implemented by the parser. guard let value = T(exactly: double) else { - throw JSONError.numberIsNotRepresentableInSwift(parsed: String(decoding: numberBuffer, as: UTF8.self)) + throw createNumberConversionError() } // The distance between Double(s) is >=2 from ±2^53. @@ -1021,7 +1037,7 @@ extension JSONDecoderImpl: Decoder { let decimalParseResult = Decimal._decimal(from: numberBuffer, matchEntireString: true).asOptional if let decimal = decimalParseResult.result { guard let value = T(decimal) else { - throw JSONError.numberIsNotRepresentableInSwift(parsed: String(decoding: numberBuffer, as: UTF8.self)) + throw createNumberConversionError() } return value } diff --git a/Tests/FoundationEssentialsTests/JSONEncoderTests.swift b/Tests/FoundationEssentialsTests/JSONEncoderTests.swift index 566919ece..74ce352d0 100644 --- a/Tests/FoundationEssentialsTests/JSONEncoderTests.swift +++ b/Tests/FoundationEssentialsTests/JSONEncoderTests.swift @@ -901,6 +901,122 @@ private struct JSONEncoderTests { _testDecodeFailure(of: DecodeFailure.self, data: toDecode.data(using: .utf8)!) } + @Test("Decimal to Int decoding error message regression test for issue #1604") + func decimalToIntDecodingErrorMessage() throws { + struct Object: Decodable { + var foo: Int + } + + let json = """ + { + "foo": 123.45 + } + """.data(using: .utf8)! + + do { + _ = try JSONDecoder().decode(Object.self, from: json) + Issue.record("Expected decoding error but decoding succeeded") + } catch let error as DecodingError { + guard case .dataCorrupted(let context) = error else { + Issue.record("Expected dataCorrupted error, got \(error)") + return + } + + // Verification 1: Check if codingPath is correctly set + // Expected: ["foo"] + #expect(context.codingPath.count == 1, "codingPath should contain one element, got \(context.codingPath)") + if let key = context.codingPath.first { + #expect(key.stringValue == "foo", "codingPath should be ['foo'], got '\(key.stringValue)'") + } + + // Verification 2: Check if debugDescription is appropriate + // Expected: A specific message like "Parsed JSON number <123.45> does not fit in Int." + #expect( + context.debugDescription.contains("123.45"), + "debugDescription should mention the number 123.45, got: \(context.debugDescription)" + ) + #expect( + context.debugDescription.contains("Int") || context.debugDescription.contains("fit"), + "debugDescription should mention Int or fit, got: \(context.debugDescription)" + ) + + // Verification 3 (macOS only): Check if underlyingError is provided + #if FOUNDATION_FRAMEWORK + #expect(context.underlyingError != nil, "macOS should provide underlyingError, got nil") + + if let nsError = context.underlyingError as? NSError { + #expect(nsError.domain == NSCocoaErrorDomain, "underlyingError domain should be NSCocoaErrorDomain") + #expect(nsError.code == CocoaError.propertyListReadCorrupt.rawValue, "underlyingError code should match propertyListReadCorrupt") + + // Verify the error message mentions the number + if let debugDesc = nsError.userInfo[NSDebugDescriptionErrorKey] as? String { + #expect(debugDesc.contains("123.45"), "underlyingError should mention the number 123.45") + } + } + #endif + } catch { + Issue.record("Expected DecodingError, got \(type(of: error)): \(error)") + } + } + + @Test("Negative to UInt decoding error message regression test for issue #1604") + func negativeToUIntDecodingErrorMessage() throws { + struct Object: Decodable { + var foo: UInt + } + + let json = """ + { + "foo": -123 + } + """.data(using: .utf8)! + + do { + _ = try JSONDecoder().decode(Object.self, from: json) + Issue.record("Expected decoding error but decoding succeeded") + } catch let error as DecodingError { + guard case .dataCorrupted(let context) = error else { + Issue.record("Expected dataCorrupted error, got \(error)") + return + } + + // Verification 1: Check if codingPath is correctly set + // Expected: ["foo"] + #expect(context.codingPath.count == 1, "codingPath should contain one element, got \(context.codingPath)") + if let key = context.codingPath.first { + #expect(key.stringValue == "foo", "codingPath should be ['foo'], got '\(key.stringValue)'") + } + + // Verification 2: Check if debugDescription is appropriate + // Expected: A specific message like "Parsed JSON number <-123> does not fit in UInt." + #expect( + context.debugDescription.contains("-123"), + "debugDescription should mention the number -123, got: \(context.debugDescription)" + ) + #expect( + context.debugDescription.contains("UInt") || context.debugDescription.contains("fit"), + "debugDescription should mention UInt or fit, got: \(context.debugDescription)" + ) + + // Verification 3 (macOS only): Check if underlyingError is provided + #if FOUNDATION_FRAMEWORK + #expect(context.underlyingError != nil, "macOS should provide underlyingError, got nil") + + if let nsError = context.underlyingError as? NSError { + #expect(nsError.domain == NSCocoaErrorDomain, "underlyingError domain should be NSCocoaErrorDomain") + #expect(nsError.code == CocoaError.propertyListReadCorrupt.rawValue, "underlyingError code should match propertyListReadCorrupt") + + // Verify the error message mentions the number + if let debugDesc = nsError.userInfo[NSDebugDescriptionErrorKey] as? String { + #expect(debugDesc.contains("-123"), "underlyingError should mention the number -123") + } + } + #endif + } catch { + Issue.record("Expected DecodingError, got \(type(of: error)): \(error)") + } + } + @Test func repeatedFailedNilChecks() { struct RepeatNilCheckDecodable : Decodable { enum Failure : Error {