Parse escape changes to Asterisk AMI VarSet events. #1
Conversation
Characters like ' " ? now have backslashes in front of them on VarSet events.
|
@gfaza Can you review this for me? |
There was a problem hiding this comment.
looks good - just a couple of comments. fwiw, i used rubocop to inform the two comments. even though it suggests a number of things in specs/ruby_ami/event_spec.rb as well, i think it best to ignore those file's suggestions in order to keep close to the adhearsion/ruby_ami.
lib/ruby_ami/event.rb
Outdated
| @@ -24,6 +39,25 @@ def best_time | |||
| timestamp || receipt_time | |||
| end | |||
|
|
|||
| # Set a Header value | |||
| # @override RubyAMI::Response | |||
| def []=(key,value) | |||
| @@ -5,6 +5,21 @@ | |||
|
|
|||
| module RubyAMI | |||
| class Event < Response | |||
| VAR_SET_ENCODING_PATTERN = /\\[abfnrtv\\'"?]/.freeze | |||
There was a problem hiding this comment.
rubocop tells me that we don't need .freeze here, b/c "Freezing immutable objects is pointless."
There was a problem hiding this comment.
I'm not getting that error with rubocop 0.74.0 and the following .rubocop.yml. Can you share the deets of the failure & rubocop version?
AllCops:
DisplayCopNames: true
DisplayStyleGuide: true
ExtraDetails: true
TargetRubyVersion: 2.3There was a problem hiding this comment.
i like your setup better than mine. seems i used ruboocp 0.41.2 in order to get this suggestion regarding the .freeze, and i certainly do not see the suggestion when using your more appropriate setup.
although i understand that a literal regex object is immutable, and shouldn't require a freeze, i'd rather defer to the results that you got - perhaps there's a good reason that the .freeze isn't superfluous when targeting ruby 2.3
There was a problem hiding this comment.
Ah, gotcha. Yeah, that makes sense. Superfluous in Ruby 1.9 but not superfluous afterwards... yeah, freezing seems safer.
Thanks man.
DTPORTAL-16626 [RUBYAMI] Ensure that ASR results from Asterisk 13.5+ can be parsed
Description
Characters like
'"and?now have backslashes in front of them on VarSet events (\'\"\?). Unescape the escaped characters so that RubyAMI exposes the original, true value of encoded channel variables.This PR partially-addresses adhearsion#47, but without the logic yet to only perform this decoding for AMI_VERSION >= 2.8.0; that will be addressed in a future PR.
Changes
This code adds/modifies/uses:
CredentialsSQL or other datastore queriesExposed API or web endpoints / URLsUser input from channels such as GET, POST, Cookie or other.Disk I/O (such as adding logging messages or creating/reading files)Checklist
Process Checklist
Security and Performance Checklist
I am @-mentioning a DBA on my PR if it includes new or modified SQL or ElasticSearch queries.