Алексей Неганов has uploaded this change for review.
mime: handling invalid mime media parameters Sometimes it's necessary to deal with emails that do not follow the specification; in particular, it's possible to download such email via gmail. When the existing implementation handle invalid mime media parameters, it returns nils and error, although there is a valid media type, which may be returned. If this behavior changes, it may not affect any existing programs, but it will help to parse some emails. Fixes #19498 Change-Id: Ieb2fdbddfd93857faee941d2aa49d59e286d57fd --- M src/mime/mediatype.go M src/mime/mediatype_test.go 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/mime/mediatype.go b/src/mime/mediatype.go
index 75cc903..758d621 100644
--- a/src/mime/mediatype.go
+++ b/src/mime/mediatype.go
@@ -94,6 +94,9 @@
return nil
}
+// ErrInvalidMediaParameter means that mime media parameter list is invalid
+var ErrInvalidMediaParameter = errors.New("mime: invalid media parameter")
+
// ParseMediaType parses a media type value and any optional
// parameters, per RFC 1521. Media types are the values in
// Content-Type and Content-Disposition headers (RFC 2183).
@@ -134,7 +137,7 @@
return
}
// Parse error.
- return "", nil, errors.New("mime: invalid media parameter")
+ return mediatype, nil, ErrInvalidMediaParameter
}
pmap := params
diff --git a/src/mime/mediatype_test.go b/src/mime/mediatype_test.go
index c5fc906..497f437 100644
--- a/src/mime/mediatype_test.go
+++ b/src/mime/mediatype_test.go
@@ -264,7 +264,7 @@
func TestParseMediaTypeBogus(t *testing.T) {
for _, tt := range badMediaTypeTests {
- mt, params, err := ParseMediaType(tt.in)
+ _, params, err := ParseMediaType(tt.in)
if err == nil {
t.Errorf("ParseMediaType(%q) = nil error; want parse error", tt.in)
continue
@@ -274,9 +274,6 @@
}
if params != nil {
t.Errorf("ParseMediaType(%q): got non-nil params on error", tt.in)
- }
- if mt != "" {
- t.Errorf("ParseMediaType(%q): got non-empty media type string on error", tt.in)
}
}
}
To view, visit change 38190. To unsubscribe, visit settings.
Ian Lance Taylor posted comments on this change.
Patch set 1:
Please add a test for the new behavior: test that a valid mediatype is returned when the parameters are erroneous, but not otherwise.
(2 comments)
Patch Set #1, Line 97: // ErrInvalidMediaParameter means that mime media parameter list is invalid
// ErrInvalidMediaParameter is returned by ParseMediaType if the media type value was found but there was an error parsing the optional parameters.
Patch Set #1, Line 107: func ParseMediaType(v string) (mediatype string, params map[string]string, err error) {
You need to update the comment to explain that mediatype can be returned even in an error case.
To view, visit change 38190. To unsubscribe, visit settings.
Алексей Неганов uploaded patch set #2 to this change.
mime: handling invalid mime media parameters Sometimes it's necessary to deal with emails that do not follow the specification; in particular, it's possible to download such email via gmail. When the existing implementation handle invalid mime media parameters, it returns nils and error, although there is a valid media type, which may be returned. If this behavior changes, it may not affect any existing programs, but it will help to parse some emails. Fixes #19498 Change-Id: Ieb2fdbddfd93857faee941d2aa49d59e286d57fd --- M src/mime/mediatype.go M src/mime/mediatype_test.go 2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/mime/mediatype.go b/src/mime/mediatype.go
index 75cc903..758d621 100644
--- a/src/mime/mediatype.go
+++ b/src/mime/mediatype.go
@@ -94,6 +94,9 @@
return nil
}
+// ErrInvalidMediaParameter means that mime media parameter list is invalid
+var ErrInvalidMediaParameter = errors.New("mime: invalid media parameter")
+
// ParseMediaType parses a media type value and any optional
// parameters, per RFC 1521. Media types are the values in
// Content-Type and Content-Disposition headers (RFC 2183).
@@ -134,7 +137,7 @@
return
}
// Parse error.
- return "", nil, errors.New("mime: invalid media parameter")
+ return mediatype, nil, ErrInvalidMediaParameter
}
pmap := params
diff --git a/src/mime/mediatype_test.go b/src/mime/mediatype_test.go
index c5fc906..66439f3 100644
--- a/src/mime/mediatype_test.go
+++ b/src/mime/mediatype_test.go
@@ -253,13 +253,18 @@
type badMediaTypeTest struct {
in string
+ mt string
err string
}
var badMediaTypeTests = []badMediaTypeTest{
- {"bogus ;=========", "mime: invalid media parameter"},
- {"bogus/<script>alert</script>", "mime: expected token after slash"},
- {"bogus/bogus<script>alert</script>", "mime: unexpected content after media subtype"},
+ {"bogus ;=========", "bogus", "mime: invalid media parameter"},
+ /* The following example is from real email delivered by gmail (error: missing semicolon)
+ and it is there to check behavior described in #19498 */
+ {"application/pdf; x-mac-type=\"3F3F3F3F\"; x-mac-creator=\"3F3F3F3F\" name=\"a.pdf\";",
+ "application/pdf", "mime: invalid media parameter"},
+ {"bogus/<script>alert</script>", "", "mime: expected token after slash"},
+ {"bogus/bogus<script>alert</script>", "", "mime: unexpected content after media subtype"},
}
func TestParseMediaTypeBogus(t *testing.T) {
@@ -275,8 +280,11 @@
if params != nil {
t.Errorf("ParseMediaType(%q): got non-nil params on error", tt.in)
}
- if mt != "" {
- t.Errorf("ParseMediaType(%q): got non-empty media type string on error", tt.in)
+ if err != ErrInvalidMediaParameter && mt != "" {
+ t.Errorf("ParseMediaType(%q): got unexpected non-empty media type string", tt.in)
+ }
+ if err == ErrInvalidMediaParameter && mt != tt.mt {
+ t.Errorf("ParseMediaType(%q): in case of invalid parameters: expected type %q, got %q", tt.in, tt.mt, mt)
}
}
}
To view, visit change 38190. To unsubscribe, visit settings.
Алексей Неганов uploaded patch set #3 to this change.
mime: handling invalid mime media parameters Sometimes it's necessary to deal with emails that do not follow the specification; in particular, it's possible to download such email via gmail. When the existing implementation handle invalid mime media parameters, it returns nils and error, although there is a valid media type, which may be returned. If this behavior changes, it may not affect any existing programs, but it will help to parse some emails. Fixes #19498 Change-Id: Ieb2fdbddfd93857faee941d2aa49d59e286d57fd --- M src/mime/mediatype.go M src/mime/mediatype_test.go 2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/mime/mediatype.go b/src/mime/mediatype.go
index 75cc903..758d621 100644
--- a/src/mime/mediatype.go
+++ b/src/mime/mediatype.go
@@ -94,6 +94,9 @@
return nil
}
+// ErrInvalidMediaParameter means that mime media parameter list is invalid
+var ErrInvalidMediaParameter = errors.New("mime: invalid media parameter")
+
// ParseMediaType parses a media type value and any optional
// parameters, per RFC 1521. Media types are the values in
// Content-Type and Content-Disposition headers (RFC 2183).
@@ -134,7 +137,7 @@
return
}
// Parse error.
- return "", nil, errors.New("mime: invalid media parameter")
+ return mediatype, nil, ErrInvalidMediaParameter
}
pmap := params
diff --git a/src/mime/mediatype_test.go b/src/mime/mediatype_test.go
index c5fc906..66439f3 100644
--- a/src/mime/mediatype_test.go
+++ b/src/mime/mediatype_test.go
@@ -253,13 +253,18 @@
type badMediaTypeTest struct {
in string
+ mt string
err string
}
var badMediaTypeTests = []badMediaTypeTest{
- {"bogus ;=========", "mime: invalid media parameter"},
- {"bogus/<script>alert</script>", "mime: expected token after slash"},
- {"bogus/bogus<script>alert</script>", "mime: unexpected content after media subtype"},
+ {"bogus ;=========", "bogus", "mime: invalid media parameter"},
+ /* The following example is from real email delivered by gmail (error: missing semicolon)
+ and it is there to check behavior described in #19498 */
+ {"application/pdf; x-mac-type=\"3F3F3F3F\"; x-mac-creator=\"3F3F3F3F\" name=\"a.pdf\";",
+ "application/pdf", "mime: invalid media parameter"},
+ {"bogus/<script>alert</script>", "", "mime: expected token after slash"},
+ {"bogus/bogus<script>alert</script>", "", "mime: unexpected content after media subtype"},
}
func TestParseMediaTypeBogus(t *testing.T) {
@@ -275,8 +280,11 @@
if params != nil {
t.Errorf("ParseMediaType(%q): got non-nil params on error", tt.in)
}
- if mt != "" {
- t.Errorf("ParseMediaType(%q): got non-empty media type string on error", tt.in)
+ if err != ErrInvalidMediaParameter && mt != "" {
+ t.Errorf("ParseMediaType(%q): got unexpected non-empty media type string", tt.in)
+ }
+ if err == ErrInvalidMediaParameter && mt != tt.mt {
+ t.Errorf("ParseMediaType(%q): in case of invalid parameters: expected type %q, got %q", tt.in, tt.mt, mt)
}
}
}
To view, visit change 38190. To unsubscribe, visit settings.
Алексей Неганов uploaded patch set #4 to this change.
mime: handling invalid mime media parameters Sometimes it's necessary to deal with emails that do not follow the specification; in particular, it's possible to download such email via gmail. When the existing implementation handle invalid mime media parameters, it returns nils and error, although there is a valid media type, which may be returned. If this behavior changes, it may not affect any existing programs, but it will help to parse some emails. Fixes #19498 Change-Id: Ieb2fdbddfd93857faee941d2aa49d59e286d57fd --- M src/mime/mediatype.go M src/mime/mediatype_test.go 2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/src/mime/mediatype.go b/src/mime/mediatype.go
index 75cc903..a5b8645 100644
--- a/src/mime/mediatype.go
+++ b/src/mime/mediatype.go
@@ -94,11 +94,18 @@
return nil
}
+// ErrInvalidMediaParameter is returned by ParseMediaType if
+// the media type value was found but there was an error parsing
+// the optional parameters
+var ErrInvalidMediaParameter = errors.New("mime: invalid media parameter")
+
// ParseMediaType parses a media type value and any optional
// parameters, per RFC 1521. Media types are the values in
// Content-Type and Content-Disposition headers (RFC 2183).
// On success, ParseMediaType returns the media type converted
// to lowercase and trimmed of white space and a non-nil map.
+// Mediatype can be returned even in error case, if there was
+// an error parsing the optional parameters.
// The returned map, params, maps from the lowercase
// attribute to the attribute value with its case preserved.
func ParseMediaType(v string) (mediatype string, params map[string]string, err error) {
@@ -134,7 +141,7 @@
return
}
// Parse error.
- return "", nil, errors.New("mime: invalid media parameter")
+ return mediatype, nil, ErrInvalidMediaParameter
}
pmap := params
diff --git a/src/mime/mediatype_test.go b/src/mime/mediatype_test.go
index c5fc906..66439f3 100644
--- a/src/mime/mediatype_test.go
+++ b/src/mime/mediatype_test.go
@@ -253,13 +253,18 @@
type badMediaTypeTest struct {
in string
+ mt string
err string
}
var badMediaTypeTests = []badMediaTypeTest{
- {"bogus ;=========", "mime: invalid media parameter"},
- {"bogus/<script>alert</script>", "mime: expected token after slash"},
- {"bogus/bogus<script>alert</script>", "mime: unexpected content after media subtype"},
+ {"bogus ;=========", "bogus", "mime: invalid media parameter"},
+ /* The following example is from real email delivered by gmail (error: missing semicolon)
+ and it is there to check behavior described in #19498 */
+ {"application/pdf; x-mac-type=\"3F3F3F3F\"; x-mac-creator=\"3F3F3F3F\" name=\"a.pdf\";",
+ "application/pdf", "mime: invalid media parameter"},
+ {"bogus/<script>alert</script>", "", "mime: expected token after slash"},
+ {"bogus/bogus<script>alert</script>", "", "mime: unexpected content after media subtype"},
}
func TestParseMediaTypeBogus(t *testing.T) {
@@ -275,8 +280,11 @@
if params != nil {
t.Errorf("ParseMediaType(%q): got non-nil params on error", tt.in)
}
- if mt != "" {
- t.Errorf("ParseMediaType(%q): got non-empty media type string on error", tt.in)
+ if err != ErrInvalidMediaParameter && mt != "" {
+ t.Errorf("ParseMediaType(%q): got unexpected non-empty media type string", tt.in)
+ }
+ if err == ErrInvalidMediaParameter && mt != tt.mt {
+ t.Errorf("ParseMediaType(%q): in case of invalid parameters: expected type %q, got %q", tt.in, tt.mt, mt)
}
}
}
To view, visit change 38190. To unsubscribe, visit settings.
Ian Lance Taylor posted comments on this change.
Patch set 4:
(2 comments)
Patch Set #4, Line 107: // Mediatype can be returned even in error case, if there was
Avoid passive voice. "If there is an error parsing the optional parameter, the media type will be returned along with the error ErrInvalidMediaParameter."
File src/mime/mediatype_test.go:
Patch Set #4, Line 262: /* The following example is from real email delivered by gmail (error: missing semicolon)
Use // comment style here.
To view, visit change 38190. To unsubscribe, visit settings.
Алексей Неганов uploaded patch set #5 to this change.
mime: handling invalid mime media parameters Sometimes it's necessary to deal with emails that do not follow the specification; in particular, it's possible to download such email via gmail. When the existing implementation handle invalid mime media parameters, it returns nils and error, although there is a valid media type, which may be returned. If this behavior changes, it may not affect any existing programs, but it will help to parse some emails. Fixes #19498 Change-Id: Ieb2fdbddfd93857faee941d2aa49d59e286d57fd --- M src/mime/mediatype.go M src/mime/mediatype_test.go 2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/mime/mediatype.go b/src/mime/mediatype.go
index 75cc903..5557672 100644
--- a/src/mime/mediatype.go
+++ b/src/mime/mediatype.go
@@ -94,11 +94,19 @@
return nil
}
+// ErrInvalidMediaParameter is returned by ParseMediaType if
+// the media type value was found but there was an error parsing
+// the optional parameters
+var ErrInvalidMediaParameter = errors.New("mime: invalid media parameter")
+
// ParseMediaType parses a media type value and any optional
// parameters, per RFC 1521. Media types are the values in
// Content-Type and Content-Disposition headers (RFC 2183).
// On success, ParseMediaType returns the media type converted
// to lowercase and trimmed of white space and a non-nil map.
+// If there is an error parsing the optional parameter,
+// the media type will be returned along with the error
+// ErrInvalidMediaParameter.
// The returned map, params, maps from the lowercase
// attribute to the attribute value with its case preserved.
func ParseMediaType(v string) (mediatype string, params map[string]string, err error) {
@@ -134,7 +142,7 @@
return
}
// Parse error.
- return "", nil, errors.New("mime: invalid media parameter")
+ return mediatype, nil, ErrInvalidMediaParameter
}
pmap := params
diff --git a/src/mime/mediatype_test.go b/src/mime/mediatype_test.go
index c5fc906..3ba8ee1 100644
--- a/src/mime/mediatype_test.go
+++ b/src/mime/mediatype_test.go
@@ -253,13 +253,18 @@
type badMediaTypeTest struct {
in string
+ mt string
err string
}
var badMediaTypeTests = []badMediaTypeTest{
- {"bogus ;=========", "mime: invalid media parameter"},
- {"bogus/<script>alert</script>", "mime: expected token after slash"},
- {"bogus/bogus<script>alert</script>", "mime: unexpected content after media subtype"},
+ {"bogus ;=========", "bogus", "mime: invalid media parameter"},
+ // The following example is from real email delivered by gmail (error: missing semicolon)
+ // and it is there to check behavior described in #19498
+ {"application/pdf; x-mac-type=\"3F3F3F3F\"; x-mac-creator=\"3F3F3F3F\" name=\"a.pdf\";",
+ "application/pdf", "mime: invalid media parameter"},
+ {"bogus/<script>alert</script>", "", "mime: expected token after slash"},
+ {"bogus/bogus<script>alert</script>", "", "mime: unexpected content after media subtype"},
}
func TestParseMediaTypeBogus(t *testing.T) {
@@ -275,8 +280,11 @@
if params != nil {
t.Errorf("ParseMediaType(%q): got non-nil params on error", tt.in)
}
- if mt != "" {
- t.Errorf("ParseMediaType(%q): got non-empty media type string on error", tt.in)
+ if err != ErrInvalidMediaParameter && mt != "" {
+ t.Errorf("ParseMediaType(%q): got unexpected non-empty media type string", tt.in)
+ }
+ if err == ErrInvalidMediaParameter && mt != tt.mt {
+ t.Errorf("ParseMediaType(%q): in case of invalid parameters: expected type %q, got %q", tt.in, tt.mt, mt)
}
}
}
To view, visit change 38190. To unsubscribe, visit settings.
Ian Lance Taylor posted comments on this change.
Patch set 5:Run-TryBot +1Code-Review +2
Thanks.
Gobot Gobot posted comments on this change.
Patch set 5:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=957b5e18
Gobot Gobot posted comments on this change.
Patch set 5:TryBot-Result +1
TryBots are happy.
Ian Lance Taylor merged this change.
mime: handling invalid mime media parameters Sometimes it's necessary to deal with emails that do not follow the specification; in particular, it's possible to download such email via gmail. When the existing implementation handle invalid mime media parameters, it returns nils and error, although there is a valid media type, which may be returned. If this behavior changes, it may not affect any existing programs, but it will help to parse some emails. Fixes #19498 Change-Id: Ieb2fdbddfd93857faee941d2aa49d59e286d57fd Reviewed-on: https://go-review.googlesource.com/38190 Reviewed-by: Ian Lance Taylor <ia...@golang.org> Run-TryBot: Ian Lance Taylor <ia...@golang.org> TryBot-Result: Gobot Gobot <go...@golang.org> --- M src/mime/mediatype.go M src/mime/mediatype_test.go 2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/mime/mediatype.go b/src/mime/mediatype.go
index 75cc903..5557672 100644
--- a/src/mime/mediatype.go
+++ b/src/mime/mediatype.go
@@ -94,11 +94,19 @@
return nil
}
+// ErrInvalidMediaParameter is returned by ParseMediaType if
+// the media type value was found but there was an error parsing
+// the optional parameters
+var ErrInvalidMediaParameter = errors.New("mime: invalid media parameter")
+
// ParseMediaType parses a media type value and any optional
// parameters, per RFC 1521. Media types are the values in
// Content-Type and Content-Disposition headers (RFC 2183).
// On success, ParseMediaType returns the media type converted
// to lowercase and trimmed of white space and a non-nil map.
+// If there is an error parsing the optional parameter,
+// the media type will be returned along with the error
+// ErrInvalidMediaParameter.
// The returned map, params, maps from the lowercase
// attribute to the attribute value with its case preserved.
func ParseMediaType(v string) (mediatype string, params map[string]string, err error) {
@@ -134,7 +142,7 @@
return
}
// Parse error.
- return "", nil, errors.New("mime: invalid media parameter")
+ return mediatype, nil, ErrInvalidMediaParameter
}
pmap := params
diff --git a/src/mime/mediatype_test.go b/src/mime/mediatype_test.go
index c5fc906..3ba8ee1 100644
--- a/src/mime/mediatype_test.go
+++ b/src/mime/mediatype_test.go
@@ -253,13 +253,18 @@
type badMediaTypeTest struct {
in string
+ mt string
err string
}
var badMediaTypeTests = []badMediaTypeTest{
- {"bogus ;=========", "mime: invalid media parameter"},
- {"bogus/<script>alert</script>", "mime: expected token after slash"},
- {"bogus/bogus<script>alert</script>", "mime: unexpected content after media subtype"},
+ {"bogus ;=========", "bogus", "mime: invalid media parameter"},
+ // The following example is from real email delivered by gmail (error: missing semicolon)
+ // and it is there to check behavior described in #19498
+ {"application/pdf; x-mac-type=\"3F3F3F3F\"; x-mac-creator=\"3F3F3F3F\" name=\"a.pdf\";",
+ "application/pdf", "mime: invalid media parameter"},
+ {"bogus/<script>alert</script>", "", "mime: expected token after slash"},
+ {"bogus/bogus<script>alert</script>", "", "mime: unexpected content after media subtype"},
}
func TestParseMediaTypeBogus(t *testing.T) {
@@ -275,8 +280,11 @@
if params != nil {
t.Errorf("ParseMediaType(%q): got non-nil params on error", tt.in)
}
- if mt != "" {
- t.Errorf("ParseMediaType(%q): got non-empty media type string on error", tt.in)
+ if err != ErrInvalidMediaParameter && mt != "" {
+ t.Errorf("ParseMediaType(%q): got unexpected non-empty media type string", tt.in)
+ }
+ if err == ErrInvalidMediaParameter && mt != tt.mt {
+ t.Errorf("ParseMediaType(%q): in case of invalid parameters: expected type %q, got %q", tt.in, tt.mt, mt)
}
}
}
To view, visit change 38190. To unsubscribe, visit settings.