You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2021/05/04 20:25:55 UTC
[GitHub] [couchdb] iilyak opened a new pull request #3544: Handle the case when md5 field is undefined
iilyak opened a new pull request #3544:
URL: https://github.com/apache/couchdb/pull/3544
## Overview
Previously the default value for md5 field was `<<>>`. This value changed to
undefined when we switch to using maps instead of erlang records.
The change break the `couch_att:to_json/4` funciton because `base64:encode/1`
cannot handle atom `undefined`.
The issue happens for inline attachments only this is why it wasn't discovered
earlier.
## Testing recommendations
1. run test suite
2. there is a manual way to test it as well
```
curl -u adm:pass -X PUT -H 'Content-Type: application/json' http://127.0.0.1:15984/test
curl -u adm:pass -X PUT -H 'Content-Type: application/json' -d '{"_id": "_design/ddd", "language": "javascript", "views": {"diet": {"map": ""}}, "_attachments": {"foo.txt": {"content_type": "text/plain", "data": "VGhpcyBpcyBhIGJhc2U2NCBlbmNvZGVkIHRleHQ="}}}' http://127.0.0.1:15984/test/_design/ddd
```
## Related Issues or Pull Requests
N/A
## Checklist
- [x] Code is written and works correctly
- [x] Changes are covered by tests
- [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] eiri commented on a change in pull request #3544: Handle the case when md5 field is undefined
Posted by GitBox <gi...@apache.org>.
eiri commented on a change in pull request #3544:
URL: https://github.com/apache/couchdb/pull/3544#discussion_r626093208
##########
File path: src/couch/src/couch_att.erl
##########
@@ -346,9 +346,14 @@ to_json(Att, OutputData, DataToFollow, ShowEncoding) ->
{<<"content_type">>, Type},
{<<"revpos">>, RevPos}
],
- DigestProp = case base64:encode(Md5) of
- <<>> -> [];
- Digest -> [{<<"digest">>, <<"md5-", Digest/binary>>}]
+ DigestProp = case Md5 of
+ <<>> ->
Review comment:
But you are changing logic here, right? Previously match for empty bin was on `base64` encode of `Md5` and here you are doing this on `Md5` itself. With this change an empty string will give you `<<"md5-">>, because `base64:encode(<<"">>) == <<>>`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] eiri commented on a change in pull request #3544: Handle the case when md5 field is undefined
Posted by GitBox <gi...@apache.org>.
eiri commented on a change in pull request #3544:
URL: https://github.com/apache/couchdb/pull/3544#discussion_r626093208
##########
File path: src/couch/src/couch_att.erl
##########
@@ -346,9 +346,14 @@ to_json(Att, OutputData, DataToFollow, ShowEncoding) ->
{<<"content_type">>, Type},
{<<"revpos">>, RevPos}
],
- DigestProp = case base64:encode(Md5) of
- <<>> -> [];
- Digest -> [{<<"digest">>, <<"md5-", Digest/binary>>}]
+ DigestProp = case Md5 of
+ <<>> ->
Review comment:
But you are changing logic here, right? Previously match for empty bin was on `base64` encode of `Md5` and here you are doing this on `Md5` itself. With this change an empty string now will give you digest `[{<<"digest">>, <<"md5-">>}]` instead of `[]`, because `base64:encode(<<"">>) == <<>>`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak merged pull request #3544: Handle the case when md5 field is undefined
Posted by GitBox <gi...@apache.org>.
iilyak merged pull request #3544:
URL: https://github.com/apache/couchdb/pull/3544
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a change in pull request #3544: Handle the case when md5 field is undefined
Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3544:
URL: https://github.com/apache/couchdb/pull/3544#discussion_r626492487
##########
File path: src/couch/src/couch_att.erl
##########
@@ -346,9 +346,14 @@ to_json(Att, OutputData, DataToFollow, ShowEncoding) ->
{<<"content_type">>, Type},
{<<"revpos">>, RevPos}
],
- DigestProp = case base64:encode(Md5) of
- <<>> -> [];
- Digest -> [{<<"digest">>, <<"md5-", Digest/binary>>}]
+ DigestProp = case Md5 of
+ <<>> ->
Review comment:
Nice catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a change in pull request #3544: Handle the case when md5 field is undefined
Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3544:
URL: https://github.com/apache/couchdb/pull/3544#discussion_r626777687
##########
File path: src/couch/src/couch_att.erl
##########
@@ -346,9 +346,14 @@ to_json(Att, OutputData, DataToFollow, ShowEncoding) ->
{<<"content_type">>, Type},
{<<"revpos">>, RevPos}
],
- DigestProp = case base64:encode(Md5) of
- <<>> -> [];
- Digest -> [{<<"digest">>, <<"md5-", Digest/binary>>}]
+ DigestProp = case Md5 of
+ <<>> ->
Review comment:
fixed in https://github.com/apache/couchdb/pull/3544/commits/ea6e208d0a30a8b9095ce38116afcc377f71429a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org