)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"30cb3a219fa2f3a53b6a5c1775bec23432803a6a","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Erik Berg \u003copenstack@slipsprogrammor.no\u003e"},{"line_number":5,"context_line":"CommitDate: 2021-06-15 16:05:32 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow for \u0027-\u0027 in property values"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Also update unit-test to cover this scenario."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"8c76e0c9_40bda45b","line":7,"updated":"2021-06-16 15:39:18.000000000","message":"Please mention this is for volume type.","commit_id":"e465c0e253e0e58182501427ae243327f735eb6a"},{"author":{"_account_id":25600,"name":"Ebbex","display_name":"ebbex","email":"openstack@slipsprogrammor.no","username":"ebbex"},"change_message_id":"50fb8b8ab5245adacd94783c33dee8f5109fbba6","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Erik Berg \u003copenstack@slipsprogrammor.no\u003e"},{"line_number":5,"context_line":"CommitDate: 2021-06-15 16:05:32 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow for \u0027-\u0027 in property values"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Also update unit-test to cover this scenario."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3c276e11_190c9326","line":7,"in_reply_to":"8c76e0c9_40bda45b","updated":"2021-06-17 09:37:25.000000000","message":"Done","commit_id":"e465c0e253e0e58182501427ae243327f735eb6a"}],"lib/puppet/provider/cinder_type/openstack.rb":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"30cb3a219fa2f3a53b6a5c1775bec23432803a6a","unresolved":true,"context_lines":[{"line_number":114,"context_line":"  end"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"  def self.pythondict2array(input)"},{"line_number":117,"context_line":"    json_input \u003d JSON.parse(input.gsub(/u\u0027([\\w-]*)\u0027/, \u0027\"\\1\"\u0027).gsub(/\u0027/, \u0027\"\u0027))"},{"line_number":118,"context_line":"    output \u003d []"},{"line_number":119,"context_line":"    json_input.each do | k, v |"},{"line_number":120,"context_line":"      output \u003d output + [\"#{k}\u003d#{v}\"]"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"ffdd0377_696a663f","line":117,"range":{"start_line":117,"start_character":4,"end_line":117,"end_character":77},"updated":"2021-06-16 15:39:18.000000000","message":"This seems to be the bug implemented by https://review.opendev.org/c/openstack/puppet-cinder/+/682936 .\n\nLooking at the code in cinder, there is no strict validation to reject characters like - or even 4 byte characters like Japaneses one.\n\nAlthough we need to consider how we handle \u0027 properly but IMO the best thing we can do is to make use [^\u0027] instead of [\\w-] so that it can accept some more \"valid\" variations.\n\nhttps://rubular.com/r/iORXhWI05P4y6U","commit_id":"e465c0e253e0e58182501427ae243327f735eb6a"},{"author":{"_account_id":25600,"name":"Ebbex","display_name":"ebbex","email":"openstack@slipsprogrammor.no","username":"ebbex"},"change_message_id":"50fb8b8ab5245adacd94783c33dee8f5109fbba6","unresolved":false,"context_lines":[{"line_number":114,"context_line":"  end"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"  def self.pythondict2array(input)"},{"line_number":117,"context_line":"    json_input \u003d JSON.parse(input.gsub(/u\u0027([\\w-]*)\u0027/, \u0027\"\\1\"\u0027).gsub(/\u0027/, \u0027\"\u0027))"},{"line_number":118,"context_line":"    output \u003d []"},{"line_number":119,"context_line":"    json_input.each do | k, v |"},{"line_number":120,"context_line":"      output \u003d output + [\"#{k}\u003d#{v}\"]"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"afb90415_ed5ead3a","line":117,"range":{"start_line":117,"start_character":4,"end_line":117,"end_character":77},"in_reply_to":"ffdd0377_696a663f","updated":"2021-06-17 09:37:25.000000000","message":"I was thinking the same about the regex, but decided against it as I wasn\u0027t sure it would be considered to greedy. But here you go 😉","commit_id":"e465c0e253e0e58182501427ae243327f735eb6a"}],"spec/unit/provider/cinder_type/openstack_spec.rb":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"35178dfe3f37369fe1947804bd0da26f41c56a11","unresolved":false,"context_lines":[{"line_number":16,"context_line":"      {"},{"line_number":17,"context_line":"         :name               \u003d\u003e \u0027Backend_1\u0027,"},{"line_number":18,"context_line":"         :ensure             \u003d\u003e :present,"},{"line_number":19,"context_line":"         :properties         \u003d\u003e [\u0027key\u003dvalue\u0027, \u0027new_key_with\u003ddashed-value\u0027, \u0027multiattach\u003d\"\u003cis\u003e True\"\u0027],"},{"line_number":20,"context_line":"         :is_public          \u003d\u003e true,"},{"line_number":21,"context_line":"         :access_project_ids \u003d\u003e [],"},{"line_number":22,"context_line":"      }"}],"source_content_type":"text/x-ruby","patch_set":3,"id":"5435cc31_a834a8d0","line":19,"range":{"start_line":19,"start_character":47,"end_line":19,"end_character":72},"updated":"2021-06-19 02:39:08.000000000","message":"I think it\u0027s better to test both _ and - instead of just replacing _ by -. This helps us to confirm that the existing behavior is not broken.","commit_id":"59e32ef904c0ba6c83dcc4d3358f5fa084c9e1f1"},{"author":{"_account_id":25600,"name":"Ebbex","display_name":"ebbex","email":"openstack@slipsprogrammor.no","username":"ebbex"},"change_message_id":"05013dadfee3b46369af4c1d40ed8993e58ad3cb","unresolved":false,"context_lines":[{"line_number":16,"context_line":"      {"},{"line_number":17,"context_line":"         :name               \u003d\u003e \u0027Backend_1\u0027,"},{"line_number":18,"context_line":"         :ensure             \u003d\u003e :present,"},{"line_number":19,"context_line":"         :properties         \u003d\u003e [\u0027key\u003dvalue\u0027, \u0027new_key_with\u003ddashed-value\u0027, \u0027multiattach\u003d\"\u003cis\u003e True\"\u0027],"},{"line_number":20,"context_line":"         :is_public          \u003d\u003e true,"},{"line_number":21,"context_line":"         :access_project_ids \u003d\u003e [],"},{"line_number":22,"context_line":"      }"}],"source_content_type":"text/x-ruby","patch_set":3,"id":"d0c6097e_9633c56e","line":19,"range":{"start_line":19,"start_character":47,"end_line":19,"end_character":72},"in_reply_to":"5435cc31_a834a8d0","updated":"2021-06-19 15:24:02.000000000","message":"Just FYI, it is/was already testing for both. The regex is a global substitution on input that looks like this;\n{u\u0027new_key_with\u0027: u\u0027dashed-value\u0027}\n\nthen you have a gsub(/u\u0027(\\w*)\u0027/, \u0027\"\\1\"\u0027) on that as a whole, giving the error;\n\nCould not prefetch cinder_type provider \u0027openstack\u0027: 751: unexpected token at \u0027{\"new_key_with\": u\u0027dashed-value\u0027}\u0027\n\nThe new_key_with underscores passes the regex, the dashed-value doesn\u0027t.\n\nWith the new regex we\u0027re not only allowing for _values_ with dashes and other characters, but also _keys_. (It just so happened that our issue was with us having a dash in the value section.)\n\nAnd I think the regex you suggested casting a wider net was the right call. We\u0027re not inserting values, we\u0027re just using what\u0027s already there. I don\u0027t think we need to do any validation of data that\u0027s already been allowed in.","commit_id":"59e32ef904c0ba6c83dcc4d3358f5fa084c9e1f1"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a448958e6470c8b44d3e819f4999150df512d043","unresolved":false,"context_lines":[{"line_number":16,"context_line":"      {"},{"line_number":17,"context_line":"         :name               \u003d\u003e \u0027Backend_1\u0027,"},{"line_number":18,"context_line":"         :ensure             \u003d\u003e :present,"},{"line_number":19,"context_line":"         :properties         \u003d\u003e [\u0027key\u003dvalue\u0027, \u0027new_key_with\u003ddashed-value\u0027, \u0027multiattach\u003d\"\u003cis\u003e True\"\u0027],"},{"line_number":20,"context_line":"         :is_public          \u003d\u003e true,"},{"line_number":21,"context_line":"         :access_project_ids \u003d\u003e [],"},{"line_number":22,"context_line":"      }"}],"source_content_type":"text/x-ruby","patch_set":3,"id":"20d3d786_de1f6c91","line":19,"range":{"start_line":19,"start_character":47,"end_line":19,"end_character":72},"in_reply_to":"d0c6097e_9633c56e","updated":"2021-06-20 13:03:28.000000000","message":"\u003e Just FYI, it is/was already testing for both. ...\nOops. You are correct and _ was already tested in key part because current regex doesn\u0027t split values by \u003d...\n\n\u003e And I think the regex you suggested casting a wider net was the right call. We\u0027re not inserting values, we\u0027re just using what\u0027s already there. I don\u0027t think we need to do any validation of data that\u0027s already been allowed in.\nI totally agree with this point. There is not reason why we need more strict restriction in puppet layer.","commit_id":"59e32ef904c0ba6c83dcc4d3358f5fa084c9e1f1"}]}