Skip to content

Commit 17b96ed

Browse files
NeedleInAJayStackNeedleInAJayStack
authored
Merge pull request #105 from NeedleInAJayStack/fix/codeSafety
Code safety improvements
2 parents 6faf27d + fa77ba8 commit 17b96ed

File tree

23 files changed

+332
-180
lines changed

23 files changed

+332
-180
lines changed

‎.github/workflows/build.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ name: Tests
22

33
on:
44
push:
5-
branches: [ master ]
5+
branches: [ main ]
66
paths-ignore: [ README.md ]
77
pull_request:
8-
branches: [ master ]
8+
branches: [ main ]
99
paths-ignore: [ README.md ]
1010
workflow_dispatch:
1111

‎Sources/GraphQL/Error/SyntaxError.swift‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,19 @@ func highlightSourceAtLocation(source: Source, location: SourceLocation) -> Stri
5050

5151
func splitLines(string: String) -> [String] {
5252

53-
let nsstring = NSString(string: string)
54-
let regex = try! NSRegularExpression(pattern: "\r\n|[\n\r]", options: [])
55-
5653
var lines: [String] = []
5754
var location = 0
58-
59-
for match in regex.matches(in: string, options: [], range: NSRange(0..<nsstring.length)) {
60-
let range = NSRange(location..<match.range.location)
61-
lines.append(nsstring.substring(with: range))
62-
location = match.range.location + match.range.length
55+
56+
let nsstring = NSString(string: string)
57+
do {
58+
let regex = try NSRegularExpression(pattern: "\r\n|[\n\r]", options: [])
59+
for match in regex.matches(in: string, options: [], range: NSRange(0..<nsstring.length)) {
60+
let range = NSRange(location..<match.range.location)
61+
lines.append(nsstring.substring(with: range))
62+
location = match.range.location + match.range.length
63+
}
64+
} catch {
65+
// Let lines and location remain unchanged
6366
}
6467

6568
if lines.isEmpty {

‎Sources/GraphQL/Execution/Execute.swift‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,7 @@ public func resolveField(
670670
let fieldAST = fieldASTs[0]
671671
let fieldName = fieldAST.name.value
672672

673-
let fieldDef = getFieldDef(
673+
let fieldDef = try getFieldDef(
674674
schema: exeContext.schema,
675675
parentType: parentType,
676676
fieldName: fieldName
@@ -796,7 +796,7 @@ func completeValueCatchingError(
796796
result: result
797797
).flatMapError { error -> EventLoopFuture<Any?> in
798798
guard let error = error as? GraphQLError else {
799-
fatalError()
799+
return exeContext.eventLoopGroup.next().makeFailedFuture(error)
800800
}
801801
exeContext.append(error: error)
802802
return exeContext.eventLoopGroup.next().makeSucceededFuture(nil)
@@ -809,7 +809,7 @@ func completeValueCatchingError(
809809
exeContext.append(error: error)
810810
return exeContext.eventLoopGroup.next().makeSucceededFuture(nil)
811811
} catch {
812-
fatalError()
812+
return exeContext.eventLoopGroup.next().makeFailedFuture(error)
813813
}
814814
}
815815

@@ -1195,15 +1195,18 @@ func getFieldDef(
11951195
schema: GraphQLSchema,
11961196
parentType: GraphQLObjectType,
11971197
fieldName: String
1198-
) -> GraphQLFieldDefinition {
1198+
) throws -> GraphQLFieldDefinition {
11991199
if fieldName == SchemaMetaFieldDef.name && schema.queryType.name == parentType.name {
12001200
return SchemaMetaFieldDef
12011201
} else if fieldName == TypeMetaFieldDef.name && schema.queryType.name == parentType.name {
12021202
return TypeMetaFieldDef
12031203
} else if fieldName == TypeNameMetaFieldDef.name {
12041204
return TypeNameMetaFieldDef
12051205
}
1206-
1207-
// we know this field exists because we passed validation before execution
1208-
return parentType.fields[fieldName]!
1206+
1207+
// This field should exist because we passed validation before execution
1208+
guard let fieldDefinition = parentType.fields[fieldName] else {
1209+
throw GraphQLError(message: "Expected field definition not found: '\(fieldName)' on '\(parentType.name)'")
1210+
}
1211+
return fieldDefinition
12091212
}

‎Sources/GraphQL/GraphQL.swift‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,11 @@ public struct GraphQLResult : Equatable, Codable, CustomStringConvertible {
3333
}
3434

3535
public var description: String {
36-
let data = try! GraphQLJSONEncoder().encode(self)
37-
return String(data: data, encoding: .utf8)!
36+
guard let data = try? GraphQLJSONEncoder().encode(self),
37+
let dataString = String(data: data, encoding: .utf8) else {
38+
return "Unable to encode GraphQLResult"
39+
}
40+
return dataString
3841
}
3942
}
4043

‎Sources/GraphQL/Language/Lexer.swift‎

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,9 @@ func advanceLexer(lexer: Lexer) throws -> Token {
3535

3636
if token.kind != .eof {
3737
repeat {
38-
token.next = try readToken(lexer: lexer, prev: token)
39-
token = token.next!
38+
let nextToken = try readToken(lexer: lexer, prev: token)
39+
token.next = nextToken
40+
token = nextToken
4041
} while token.kind == .comment
4142

4243
lexer.token = token
@@ -608,7 +609,14 @@ func readString(source: Source, start: Int, line: Int, col: Int, prev: Token) th
608609
positionIndex = body.utf8.index(after: positionIndex)
609610

610611
if code == 92 { // \
611-
value += String(body.utf8[chunkStartIndex..<startIterationIndex])!
612+
guard let chunk = String(body.utf8[chunkStartIndex..<startIterationIndex]) else {
613+
throw syntaxError(
614+
source: source,
615+
position: body.offset(of: positionIndex),
616+
description: "Unable to initialize String from chunk: \(body.utf8[chunkStartIndex..<startIterationIndex])."
617+
)
618+
}
619+
value += chunk
612620
currentCode = body.charCode(at: positionIndex)
613621

614622
if let code = currentCode {
@@ -643,8 +651,16 @@ func readString(source: Source, start: Int, line: Int, col: Int, prev: Token) th
643651
"\\u\(body.utf8[aIndex...dIndex])."
644652
)
645653
}
646-
647-
value += String(Character(UnicodeScalar(UInt32(charCode))!))
654+
655+
guard let unicodeScalar = UnicodeScalar(UInt32(charCode)) else {
656+
throw syntaxError(
657+
source: source,
658+
position: body.offset(of: positionIndex),
659+
description:
660+
"Invalid unicode character code: \\u\(charCode)."
661+
)
662+
}
663+
value += String(Character(unicodeScalar))
648664

649665
positionIndex = dIndex
650666
default:
@@ -668,8 +684,15 @@ func readString(source: Source, start: Int, line: Int, col: Int, prev: Token) th
668684
description: "Unterminated string."
669685
)
670686
}
671-
672-
value += String(body.utf8[chunkStartIndex..<positionIndex])!
687+
688+
guard let chunk = String(body.utf8[chunkStartIndex..<positionIndex]) else {
689+
throw syntaxError(
690+
source: source,
691+
position: body.offset(of: positionIndex),
692+
description: "Unable to initialize String from chunk: \(body.utf8[chunkStartIndex..<positionIndex])."
693+
)
694+
}
695+
value += chunk
673696

674697
return Token(
675698
kind: .string,
@@ -702,7 +725,15 @@ func readBlockString(lexer: Lexer, source: Source, start: Int, line: Int, col: I
702725
body.utf8[body.utf8.index(positionIndex, offsetBy: 1)] == 34,
703726
body.utf8[body.utf8.index(positionIndex, offsetBy: 2)] == 34 {
704727

705-
rawValue += String(body.utf8[chunkStartIndex..<positionIndex])!
728+
guard let chunk = String(body.utf8[chunkStartIndex..<positionIndex]) else {
729+
throw syntaxError(
730+
source: source,
731+
position: body.offset(of: positionIndex),
732+
description: "Unable to initialize String from chunk: \(body.utf8[chunkStartIndex..<positionIndex])."
733+
)
734+
}
735+
rawValue += chunk
736+
706737
return Token(
707738
kind: .blockstring,
708739
start: start,
@@ -747,7 +778,15 @@ func readBlockString(lexer: Lexer, source: Source, start: Int, line: Int, col: I
747778
body.utf8[body.utf8.index(positionIndex, offsetBy: 2)] == 34,
748779
body.utf8[body.utf8.index(positionIndex, offsetBy: 3)] == 34 {
749780
// escaped triple quote (\""")
750-
rawValue += String(body.utf8[chunkStartIndex..<positionIndex])! + "\"\"\""
781+
782+
guard let chunk = String(body.utf8[chunkStartIndex..<positionIndex]) else {
783+
throw syntaxError(
784+
source: source,
785+
position: body.offset(of: positionIndex),
786+
description: "Unable to initialize String from chunk: \(body.utf8[chunkStartIndex..<positionIndex])."
787+
)
788+
}
789+
rawValue += chunk + "\"\"\""
751790
positionIndex = body.utf8.index(positionIndex, offsetBy: 4)
752791
chunkStartIndex = positionIndex
753792
} else {

‎Sources/GraphQL/Language/Location.swift‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,15 @@ func getLocation(source: Source, position: Int) -> SourceLocation {
1818
var line = 1
1919
var column = position + 1
2020

21-
let regex = try! NSRegularExpression(pattern: "\r\n|[\n\r]", options: [])
22-
23-
let matches = regex.matches(in: source.body, options: [], range: NSRange(0..<source.body.utf16.count))
24-
25-
for match in matches where match.range.location < position {
26-
line += 1
27-
column = position + 1 - (match.range.location + match.range.length)
21+
do {
22+
let regex = try NSRegularExpression(pattern: "\r\n|[\n\r]", options: [])
23+
let matches = regex.matches(in: source.body, options: [], range: NSRange(0..<source.body.utf16.count))
24+
for match in matches where match.range.location < position {
25+
line += 1
26+
column = position + 1 - (match.range.location + match.range.length)
27+
}
28+
} catch {
29+
// Leave line and position unset if regex fails
2830
}
2931

3032
return SourceLocation(line: line, column: column)

‎Sources/GraphQL/Language/Parser.swift‎

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,12 @@ func parseType(source: Source, noLocation: Bool = false) throws -> Type {
118118
*/
119119
func parseName(lexer: Lexer) throws -> Name {
120120
let token = try expect(lexer: lexer, kind: .name)
121+
guard let value = token.value else {
122+
throw GraphQLError(message: "Expected name token to have value: \(token)")
123+
}
121124
return Name(
122125
loc: loc(lexer: lexer, startToken: token),
123-
value: token.value!
126+
value: value
124127
)
125128
}
126129

@@ -172,8 +175,10 @@ func parseDefinition(lexer: Lexer) throws -> Definition {
172175
}
173176

174177
if peek(lexer: lexer, kind: .name) {
175-
switch lexer.token.value! {
176-
// Note: subscription is an experimental non-spec addition.
178+
guard let value = lexer.token.value else {
179+
throw GraphQLError(message: "Expected name token to have value: \(lexer.token)")
180+
}
181+
switch value {
177182
case "query", "mutation", "subscription":
178183
return try parseOperationDefinition(lexer: lexer);
179184
case "fragment":
@@ -236,11 +241,13 @@ func parseOperationDefinition(lexer: Lexer) throws -> OperationDefinition {
236241
*/
237242
func parseOperationType(lexer: Lexer) throws -> OperationType {
238243
let operationToken = try expect(lexer: lexer, kind: .name)
244+
guard let value = operationToken.value else {
245+
throw GraphQLError(message: "Expected name token to have value: \(operationToken)")
246+
}
239247

240-
switch operationToken.value! {
248+
switch value {
241249
case "query": return .query
242250
case "mutation": return .mutation
243-
// Note: subscription is an experimental non-spec addition.
244251
case "subscription": return .subscription
245252
default: throw unexpected(lexer: lexer, atToken: operationToken)
246253
}
@@ -458,26 +465,35 @@ func parseValueLiteral(lexer: Lexer, isConst: Bool) throws -> Value {
458465
return try parseObject(lexer: lexer, isConst: isConst)
459466
case .int:
460467
try lexer.advance()
468+
guard let value = token.value else {
469+
throw GraphQLError(message: "Expected int token to have value: \(token)")
470+
}
461471
return IntValue(
462472
loc: loc(lexer: lexer, startToken: token),
463-
value: token.value!
473+
value: value
464474
)
465475
case .float:
466476
try lexer.advance()
477+
guard let value = token.value else {
478+
throw GraphQLError(message: "Expected float token to have value: \(token)")
479+
}
467480
return FloatValue(
468481
loc: loc(lexer: lexer, startToken: token),
469-
value: token.value!
482+
value: value
470483
)
471484
case .string, .blockstring:
472485
return try parseStringLiteral(lexer: lexer, startToken: token)
473486
case .name:
474-
if (token.value == "true" || token.value == "false") {
487+
guard let value = token.value else {
488+
throw GraphQLError(message: "Expected name token to have value: \(token)")
489+
}
490+
if (value == "true" || value == "false") {
475491
try lexer.advance()
476492
return BooleanValue(
477493
loc: loc(lexer: lexer, startToken: token),
478-
value: token.value == "true"
494+
value: value == "true"
479495
)
480-
} else if token.value == "null" {
496+
} else if value == "null" {
481497
try lexer.advance()
482498
return NullValue(
483499
loc: loc(lexer: lexer, startToken: token)
@@ -486,7 +502,7 @@ func parseValueLiteral(lexer: Lexer, isConst: Bool) throws -> Value {
486502
try lexer.advance()
487503
return EnumValue(
488504
loc: loc(lexer: lexer, startToken: token),
489-
value: token.value!
505+
value: value
490506
)
491507
}
492508
case .dollar:
@@ -564,10 +580,13 @@ func parseObjectField(lexer: Lexer, isConst: Bool) throws -> ObjectField {
564580
*/
565581

566582
func parseStringLiteral(lexer: Lexer, startToken: Token) throws -> StringValue {
567-
try lexer.advance();
583+
try lexer.advance()
584+
guard let value = startToken.value else {
585+
throw GraphQLError(message: "Expected string literal token to have value: \(startToken)")
586+
}
568587
return StringValue(
569588
loc: loc(lexer: lexer, startToken: startToken),
570-
value: startToken.value!,
589+
value: value,
571590
block: startToken.kind == .blockstring
572591
)
573592
}
@@ -668,7 +687,10 @@ func parseTypeSystemDefinition(lexer: Lexer) throws -> TypeSystemDefinition {
668687
: lexer.token
669688

670689
if keywordToken.kind == .name {
671-
switch keywordToken.value! {
690+
guard let value = keywordToken.value else {
691+
throw GraphQLError(message: "Expected keyword token to have value: \(keywordToken)")
692+
}
693+
switch value {
672694
case "schema": return try parseSchemaDefinition(lexer: lexer);
673695
case "scalar": return try parseScalarTypeDefinition(lexer: lexer);
674696
case "type": return try parseObjectTypeDefinition(lexer: lexer);

‎Sources/GraphQL/Map/AnySerialization.swift‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,15 @@ public struct AnySerialization {
66
}
77

88
static func object(with map: Any) throws -> NSObject {
9-
return map as! NSObject
9+
guard let result = map as? NSObject else {
10+
throw EncodingError.invalidValue(
11+
map,
12+
EncodingError.Context(
13+
codingPath: [],
14+
debugDescription: "Expected object input to be castable to NSObject: \(type(of: map))"
15+
)
16+
)
17+
}
18+
return result
1019
}
1120
}

‎Sources/GraphQL/Map/Map.swift‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,13 @@ extension Map : Codable {
663663

664664
switch self {
665665
case .undefined:
666-
fatalError("undefined values should have been excluded from encoding")
666+
throw EncodingError.invalidValue(
667+
self,
668+
EncodingError.Context(
669+
codingPath: [],
670+
debugDescription: "undefined values should have been excluded from encoding"
671+
)
672+
)
667673
case .null:
668674
try container.encodeNil()
669675
case let .bool(value):
@@ -681,7 +687,15 @@ extension Map : Codable {
681687
var container = encoder.container(keyedBy: _DictionaryCodingKey.self)
682688
for (key, value) in dictionary {
683689
if !value.isUndefined {
684-
let codingKey = _DictionaryCodingKey(stringValue: key)!
690+
guard let codingKey = _DictionaryCodingKey(stringValue: key) else {
691+
throw EncodingError.invalidValue(
692+
self,
693+
EncodingError.Context(
694+
codingPath: [],
695+
debugDescription: "codingKey not found for dictionary key: \(key)"
696+
)
697+
)
698+
}
685699
try container.encode(value, forKey: codingKey)
686700
}
687701
}

0 commit comments

Comments
 (0)