Fixes an issue with aux buffer init overwriting optional parameters in receive(). #334
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n receive(). This is the same issue that was fixed in MIME code in f329aae. Closes lunarmodules#331.
|
+1 |
1 similar comment
|
+1 |
Zash
reviewed
Oct 17, 2021
greg-el
added a commit
to snowplow/snowplow-lua-tracker
that referenced
this pull request
Feb 25, 2022
- Bump supported lua versions Lua version 5.1 and up are all widely used across the Lua ecosystem, therefor supporting Lua >= 5.1 is preferable. - Replace luasocket with lua-curl Luasocket is likely a simplier option, as on mac/brew, the CURL_DIR flag is required for luacurl to be able to find curl, however luasocket currently cannot make HTTP requests with Lua >= 5.4.3: lunarmodules/luasocket#334 So for the mean time, lua-curl is a good alternative. - Replace bit + base64 modules with base64 package The bit module, which base64 relied on, was throwing an error while running the base64 test file. As these were files copied from github with no newer versions, both lib/bit.lua and lib/base64.lua have been replaced with the base64 package: https://github.com/iskolbin/lbase64 There are two differences: 1. The base64 package adds padding, whereas the previous implementation didn't - this doesn't matter however, as the collector doesn't care if padding is present in base64 2. The previous implemenation threw an error when trying to encode an empty string, this behaviour has been replaced by checking for an empty string on the variable passed into base64.encode
greg-el
added a commit
to snowplow/snowplow-lua-tracker
that referenced
this pull request
Feb 25, 2022
- Bump supported lua versions Lua version 5.1 and up are all widely used across the Lua ecosystem, therefor supporting Lua >= 5.1 is preferable. - Replace luasocket with lua-curl Luasocket is likely a simplier option, as on mac/brew, the CURL_DIR flag is required for luacurl to be able to find curl, however luasocket currently cannot make HTTP requests with Lua >= 5.4.3: lunarmodules/luasocket#334 lua-curl is a suitable alternative. - Replace bit + base64 modules with base64 package The bit module, which base64 relied on, was throwing an error while running the base64 test file. As these were files copied from github with no newer versions, both lib/bit.lua and lib/base64.lua have been replaced with the base64 package: https://github.com/iskolbin/lbase64 There are two differences: 1. The base64 package adds padding, whereas the previous implementation didn't - this doesn't matter however, as the collector doesn't care if padding is present in base64 2. The previous implemenation threw an error when trying to encode an empty string, this behaviour has been replaced by checking for an empty string on the variable passed into base64.encode
greg-el
added a commit
to snowplow/snowplow-lua-tracker
that referenced
this pull request
Feb 25, 2022
Bump supported lua versions Lua version 5.1 and up are all widely used across the Lua ecosystem, therefor supporting Lua >= 5.1 is preferable. Replace luasocket with lua-curl Luasocket is likely a simplier option, as on mac/brew, the CURL_DIR flag is required for luacurl to be able to find curl, however luasocket currently cannot make HTTP requests with Lua >= 5.4.3: lunarmodules/luasocket#334 So for the mean time, lua-curl is a good alternative. Replace bit + base64 modules with base64 package The bit module, which base64 relied on, was throwing an error while running the base64 test file. As these were files copied from github with no newer versions, both lib/bit.lua and lib/base64.lua have been replaced with the base64 package: https://github.com/iskolbin/lbase64 There are two differences: 1. The base64 package adds padding, whereas the previous implementation didn't - this doesn't matter however, as the collector doesn't care if padding is present in base64 2. The previous implemenation threw an error when trying to encode an empty string, this behaviour has been replaced by checking for an empty string on the variable passed into base64.encode Update set and fix tests - Make :toString() return in key sorted order, for use in error message comparison
greg-el
added a commit
to snowplow/snowplow-lua-tracker
that referenced
this pull request
Feb 25, 2022
Bump supported lua versions Lua version 5.1 and up are all widely used across the Lua ecosystem, therefor supporting Lua >= 5.1 is preferable. Replace luasocket with lua-curl Luasocket is likely a simplier option, as on mac/brew, the CURL_DIR flag is required for luacurl to be able to find curl, however luasocket currently cannot make HTTP requests with Lua >= 5.4.3: lunarmodules/luasocket#334 So for the mean time, lua-curl is a good alternative. Replace bit + base64 modules with base64 package The bit module, which base64 relied on, was throwing an error while running the base64 test file. As these were files copied from github with no newer versions, both lib/bit.lua and lib/base64.lua have been replaced with the base64 package: https://github.com/iskolbin/lbase64 There are two differences: 1. The base64 package adds padding, whereas the previous implementation didn't - this doesn't matter however, as the collector doesn't care if padding is present in base64 2. The previous implemenation threw an error when trying to encode an empty string, this behaviour has been replaced by checking for an empty string on the variable passed into base64.encode Update set and fix tests - Make :toString() return in key sorted order, for use in error message comparison
greg-el
added a commit
to snowplow/snowplow-lua-tracker
that referenced
this pull request
Feb 25, 2022
Bump supported lua versions Lua version 5.1 and up are all widely used across the Lua ecosystem, therefor supporting Lua >= 5.1 is preferable. Replace luasocket with lua-curl Luasocket is likely a simplier option, as on mac/brew, the CURL_DIR flag is required for luacurl to be able to find curl, however luasocket currently cannot make HTTP requests with Lua >= 5.4.3: lunarmodules/luasocket#334 lua-curl is a good alternative. Replace bit + base64 modules with base64 package The bit module, which base64 relied on, was throwing an error while running the base64 test file. As these were files copied from github with no newer versions, both lib/bit.lua and lib/base64.lua have been replaced with the base64 package: https://github.com/iskolbin/lbase64 There are two differences: 1. The base64 package adds padding, whereas the previous implementation didn't - this doesn't matter however, as the collector doesn't care if padding is present in base64 2. The previous implemenation threw an error when trying to encode an empty string, this behaviour has been replaced by checking for an empty string on the variable passed into base64.encode Update set and fix tests Make :toString() return in key sorted order, for use in error message comparison
alerque
approved these changes
Mar 18, 2022
There was a problem hiding this comment.
I've tested this across several systems (all Linux but a mix of Ubuntu & Arch Linux) and versions of Lua and this does appear to fix the bug it claims to fix. Given the limited breadth that I can test but the certain knowledge that this has completely blocked usage on current Lua (5.4.3 and 5.4.4) for Linux users I think it's fair to go ahead with a merge and release. If we must iterate past that then we must.
This was referenced Mar 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
This is the same issue that was fixed in MIME code in f329aae.
Closes #331.