Skip to content

tsung fails in ts_dynvars:merge when arguments are [<<>>, Dynvars]#258

Merged
tisba merged 1 commit into
processone:developfrom
izinin:unsafe_dynvars_merge_fix
Jun 11, 2018
Merged

tsung fails in ts_dynvars:merge when arguments are [<<>>, Dynvars]#258
tisba merged 1 commit into
processone:developfrom
izinin:unsafe_dynvars_merge_fix

Conversation

@izinin

@izinin izinin commented Sep 20, 2017

Copy link
Copy Markdown
Contributor

Hi!

ts_dynvars:merge function implementation is unsafe because it does not provide alternative for error case.

For example, in case if 'dynvars' enabled and there is no variable substitutions detected it crashes because it fails to merge empty buffer with existed Dynvars. This happens when i mix variable substitution on and off in different requests in one transaction

Regards,
Igor

@tisba

tisba commented Nov 1, 2017

Copy link
Copy Markdown
Collaborator

I'm not sure I can follow this completly. When exactly can this happen? And why is it okay for DynVars2 to always "win", if one of DynVars1 or DynVars2 is not a list?

@izinin

izinin commented Nov 1, 2017

Copy link
Copy Markdown
Contributor Author

Hi Sebastian
It crashes when the function receives empty value for 'DynVars1 ' argument. If there is nothing to merge you should not crash. Just make it safe.

I run fixed version on my branch , i exercise a proprietary protocol plugin with Tsung.

@tisba

tisba commented Jan 4, 2018

Copy link
Copy Markdown
Collaborator

I guess this is good to go. What do you think, @nniclausse?

@tisba tisba added the kind/bug label May 30, 2018
@tisba

tisba commented May 30, 2018

Copy link
Copy Markdown
Collaborator

@izinin I'd like to get this merged. Could you please change the target branch to develop? Thanks!

@tisba
tisba changed the base branch from master to develop June 11, 2018 10:45
@tisba

tisba commented Jun 11, 2018

Copy link
Copy Markdown
Collaborator

Never mind. Just realised, that I can change the base branch myself :)

@tisba
tisba merged commit 89fff07 into processone:develop Jun 11, 2018
@nniclausse nniclausse added this to the 1.8.0 milestone Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants