Skip to content

Commit 3abb3be

Browse files
[awslogs_encoding] Parse invalid request field in ELB access logs (#44475)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Parses invalid requests logged by ELB logs <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes #44233 <!--Describe what testing was performed and which tests were added.--> #### Testing Added tests for function `parseRequestField(raw string) (method, uri, protoName, protoVersion string, err error)` --------- Signed-off-by: Kavindu Dodanduwa <[email protected]> Co-authored-by: Kavindu Dodanduwa <[email protected]> Co-authored-by: Kavindu Dodanduwa <[email protected]>
1 parent 9df7f9e commit 3abb3be

File tree

3 files changed

+126
-10
lines changed

3 files changed

+126
-10
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
7+
component: extension/awslogs_encoding
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: "Fix ALB log `request_line` parsing for valid formats and avoid errors"
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [44233]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

extension/encoding/awslogsencodingextension/internal/unmarshaler/elb-access-log/elb.go

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -488,32 +488,40 @@ func extractFields(logLine string) ([]string, error) {
488488
func parseRequestField(raw string) (method, uri, protoName, protoVersion string, err error) {
489489
method, remaining, _ := strings.Cut(raw, " ")
490490
if method == "" {
491-
err = fmt.Errorf("unexpected: request field %q has no method", raw)
491+
err = fmt.Errorf("unexpected: field %q has no method section", raw)
492492
return method, uri, protoName, protoVersion, err
493493
}
494494

495-
uri, remaining, _ = strings.Cut(remaining, " ")
496-
if uri == "" {
497-
err = fmt.Errorf("unexpected: request field %q has no URI", raw)
498-
return method, uri, protoName, protoVersion, err
499-
}
495+
var protocol string
500496

501-
protocol, leftover, _ := strings.Cut(remaining, " ")
502-
if protocol == "" || leftover != "" {
503-
err = fmt.Errorf(`request field %q does not match expected format "<method> <uri> <protocol>"`, raw)
497+
index := strings.LastIndex(remaining, " ")
498+
switch {
499+
case index == -1:
500+
err = fmt.Errorf("unexpected: field %q has no protocol/version section", raw)
504501
return method, uri, protoName, protoVersion, err
502+
case index == len(remaining)-1:
503+
uri = strings.TrimSpace(remaining)
504+
protocol = unknownField
505+
default:
506+
uri = remaining[:index]
507+
protocol = remaining[index+1:]
505508
}
506509

507510
protoName, protoVersion, err = netProtocol(protocol)
508511
if err != nil {
509512
err = fmt.Errorf("invalid protocol in request field: %w", err)
510513
return method, uri, protoName, protoVersion, err
511514
}
512-
return method, uri, protoName, protoVersion, err
515+
516+
return method, uri, protoName, protoVersion, nil
513517
}
514518

515519
// netProtocol returns protocol name and version based on proto value
516520
func netProtocol(proto string) (string, string, error) {
521+
if proto == unknownField {
522+
return unknownField, unknownField, nil
523+
}
524+
517525
name, version, found := strings.Cut(proto, "/")
518526
if !found || name == "" || version == "" {
519527
return "", "", errors.New(`request uri protocol does not follow expected scheme "<name>/<version>"`)

extension/encoding/awslogsencodingextension/internal/unmarshaler/elb-access-log/elb_test.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,84 @@ func Test_scanField(t *testing.T) {
5959
})
6060
}
6161
}
62+
63+
func Test_parseRequestField(t *testing.T) {
64+
tests := []struct {
65+
name string
66+
input string
67+
wantMethod string
68+
wantURI string
69+
wantProtoName string
70+
wantProtoVersion string
71+
wantErr bool
72+
}{
73+
{
74+
name: "Valid input with expected sections",
75+
input: "GET http://example.com/ HTTP/1.1",
76+
wantMethod: "GET",
77+
wantURI: "http://example.com/",
78+
wantProtoName: "http",
79+
wantProtoVersion: "1.1",
80+
},
81+
{
82+
name: "Missing protocol/version",
83+
input: "GET http://example.com/ -",
84+
wantMethod: "GET",
85+
wantURI: "http://example.com/",
86+
wantProtoName: "-",
87+
wantProtoVersion: "-",
88+
},
89+
{
90+
name: "URI section with spaces",
91+
input: "GET http://example.com/path to somewhere HTTP/1.1",
92+
wantMethod: "GET",
93+
wantURI: "http://example.com/path to somewhere",
94+
wantProtoName: "http",
95+
wantProtoVersion: "1.1",
96+
},
97+
{
98+
name: "Input with spaces and missing protocol/version",
99+
input: "- http://example.com/path to somewhere- -",
100+
wantMethod: "-",
101+
wantURI: "http://example.com/path to somewhere-",
102+
wantProtoName: "-",
103+
wantProtoVersion: "-",
104+
},
105+
{
106+
name: "Missing method and protocol/version",
107+
input: "- http://example.com:80- ",
108+
wantErr: false,
109+
wantMethod: "-",
110+
wantURI: "http://example.com:80-",
111+
wantProtoName: "-",
112+
wantProtoVersion: "-",
113+
},
114+
{
115+
name: "Invalid input with missing expected sections",
116+
input: "GET /",
117+
wantMethod: "GET",
118+
wantErr: true,
119+
},
120+
}
121+
for _, tt := range tests {
122+
t.Run(tt.name, func(t *testing.T) {
123+
gotMethod, gotURI, gotProtoName, gotProtoVersion, err := parseRequestField(tt.input)
124+
if (err != nil) != tt.wantErr {
125+
t.Errorf("parseRequestField() error = %v, wantErr %v", err, tt.wantErr)
126+
return
127+
}
128+
if gotMethod != tt.wantMethod {
129+
t.Errorf("parseRequestField() gotMethod = %v, want %v", gotMethod, tt.wantMethod)
130+
}
131+
if gotURI != tt.wantURI {
132+
t.Errorf("parseRequestField() gotURI = %v, want %v", gotURI, tt.wantURI)
133+
}
134+
if gotProtoName != tt.wantProtoName {
135+
t.Errorf("parseRequestField() gotProtoName = %v, want %v", gotProtoName, tt.wantProtoName)
136+
}
137+
if gotProtoVersion != tt.wantProtoVersion {
138+
t.Errorf("parseRequestField() gotProtoVersion = %v, want %v", gotProtoVersion, tt.wantProtoVersion)
139+
}
140+
})
141+
}
142+
}

0 commit comments

Comments
 (0)