On 3 November 2017 at 15:47, Sam Ruby <ru...@intertwingly.net> wrote:
> On Fri, Nov 3, 2017 at 11:37 AM, Craig Russell <apache....@gmail.com> wrote:
>> I cannot reproduce this error on my local machine running Chrome or Safari.
>>
>> With the production code, the problem shows up if the Full name has the
>> non-ascii character.
>>
>> So I can't test to see if substituting \r\n for \n helps. :(
>
> Problem occurs with the latest mail gem. See what "gem list mail"
> returns. If it doesn't return 2.7.0 in the list, try running 'bundle
> update' and then "gem list mail" again.
>
>> Craig
>>
>>> On Nov 3, 2017, at 7:50 AM, Craig Russell <apache....@gmail.com> wrote:
>>>
>>>>
>>>> On Nov 2, 2017, at 10:35 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>>>>
>>>> On Thu, Nov 2, 2017 at 11:34 PM, Craig Russell <apache....@gmail.com>
>>>> wrote:
>>>>>
>>>>>> On Nov 2, 2017, at 4:24 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>>>>>>
>>>>>> On Thu, Nov 2, 2017 at 6:58 PM, Craig Russell <apache....@gmail.com>
>>>>>> wrote:
>>>>>>> The error appears to be here:
>>>>>>>
>>>>>>> # untaint to email addresses
>>>>>>> mail.to = mail.to.map {|email| email.dup.untaint} <== here
>>>>>>>
>>>>>>> But the mail.to should be r...@apache.org
>>>>>>
>>>>>> mail.to is normally an array of values. In this case, it is a string
>>>>>> containing the bulk of the headers and body of the message.
>>>>>
>>>>> Why does email to: contain all that stuff?
>>>>
>>>> Properly formatted emails terminate lines in \r\n. The template ends
>>>> lines with \n.
>>>
>>> The template code:
>>>
>>> def template(name)
>>> path = File.expand_path("../templates/#{name}", __FILE__.untaint)
>>> ERB.new(File.read(path.untaint).untaint).result(binding)
>>> end
>>>
>>> Could this be changed to replace all \n with \r\n here?
>
> my concern is that template is used in other places.
So why not change the template itself to use CRLF?
> I've created a pull request for the mail gem:
>
> https://github.com/mikel/mail/pull/1168
>
> And monkey-patched the mail gem within the workbench tool:
>
> https://github.com/apache/whimsy/commit/465cc35232ed1c9a9d8f98d1150d25081b844884
>
>>> Craig
>
> - Sam Ruby
>
>>>> Properly formatted emails also are pure ASCII; emails have a strange
>>>> convention for escaping non-ASCII characters.
>>>>
>>>> Previous versions of the mail gem recovered from both cases. Recent
>>>> changes made it so that it will recover from one or the other, but not
>>>> both simultaneously. I plan to build a pull request for the mail gem
>>>> tomorrow to address this.
>>>>
>>>>> Given that the email to: was created in line 221, I don't understand why
>>>>> it needs to be untainted.
>>>>>>
>>>>>>> Why does this need to be untainted? Why does it fail just on this
>>>>>>> email? The only thing different about this email is the non-ascii
>>>>>>> characters in the name...
>>>>>>
>>>>>> Bug reported against mail gem:
>>>>>> https://github.com/mikel/mail/issues/1167. The existence of non-ASCII
>>>>>> characters and the absence of CR's appear to be involved. I want to
>>>>>> think briefly about whether it would be better to pin to an older
>>>>>> version of this gem (which would work, but would mean that we wouldn't
>>>>>> get bug fixes), or to find a reasonable workaround.
>>>>>
>>>>> What bad would happen if line 232 were removed? Could the
>>>>> template('acreq.erb') be untainted by itself? mail cc: is untainted in
>>>>> line 229.
>>>>
>>>> https://lists.apache.org/thread.html/cf99af41102e58ffe3e3e98afa1d9861590c232373f92f079341fe3f@%3Cdev.whimsical.apache.org%3E
>>>>
>>>> There may indeed be other ways to untaint this value, perhaps
>>>> untainting the whole template might indeed work; but first we need
>>>> either have the template produce a more RFC compliant email or to make
>>>> the mail gem handle what the template does produce. As it stands now,
>>>> the mail gem will take the entire rest of the file and store it into
>>>> the to: address, which will fail later even if this were untainted.
>>>>
>>>>> Craig
>>>>>
>>>>>>
>>>>>>> Craig
>>>>>>
>>>>>> - Sam Ruby
>>>>
>>>> - Sam Ruby
>>>>
>>>>>>>> On Nov 2, 2017, at 3:31 PM, Craig Russell <apache....@gmail.com> wrote:
>>>>>>>>
>>>>>>>> Hi Sam,
>>>>>>>>
>>>>>>>>> On Nov 2, 2017, at 12:08 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>>>>>>>>>
>>>>>>>>> Reproduction instructions?
>>>>>>>>
>>>>>>>> Try to file the icla from Nandor Kollar.
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>> Craig
>>>>>>>>>
>>>>>>>>> - Sam Ruby
>>>>>>>>>
>>>>>>>>> On Thu, Nov 2, 2017 at 11:27 AM, Craig Russell <apache....@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>> #<NoMethodError: undefined method `map' for
>>>>>>>>>> #<String:0x007f9ba34add28> Did you mean? tap>
>>>>>>>>>> /x1/srv/whimsy/www/secretary/workbench/views/actions/icla.json.rb:232:in
>>>>>>>>>> `block in _evaluate'
>>>>>>>>>> /x1/srv/whimsy/www/secretary/workbench/tasks.rb:9:in `task'
>>>>>>>>>> /x1/srv/whimsy/www/secretary/workbench/views/actions/icla.json.rb:221:in
>>>>>>>>>> `_evaluate'
>>>>>>>>>> /x1/srv/whimsy/www/secretary/workbench/server.rb:68:in `block in
>>>>>>>>>> <top (required)>'
>>>>>>>>>> /x1/srv/whimsy/lib/whimsy/asf/rack.rb:223:in `call'
>>>>>>>>>> /usr/local/rvm/gems/ruby-2.4.1/gems/passenger-5.1.8/src/ruby_supportlib/phusion_passenger/rack/out_of_band_gc.rb:48:in
>>>>>>>>>> `call'
>>>>>>>>>> /x1/srv/whimsy/lib/whimsy/asf/rack.rb:148:in `call'
>>>>>>>>>> /x1/srv/whimsy/lib/whimsy/asf/rack.rb:79:in `call'
>>>>>>>>>> /x1/srv/whimsy/lib/whimsy/asf/rack.rb:254:in `call'
>>>>>>>>>> /usr/local/rvm/gems/ruby-2.4.1/gems/passenger-5.1.8/src/ruby_supportlib/phusion_passenger/rack/thread_handler_extension.rb:97:in
>>>>>>>>>> `process_request'
>>>>>>>>>> /usr/local/rvm/gems/ruby-2.4.1/gems/passenger-5.1.8/src/ruby_supportlib/phusion_passenger/request_handler/thread_handler.rb:160:in
>>>>>>>>>> `accept_and_process_next_request'
>>>>>>>>>> /usr/local/rvm/gems/ruby-2.4.1/gems/passenger-5.1.8/src/ruby_supportlib/phusion_passenger/request_handler/thread_handler.rb:113:in
>>>>>>>>>> `main_loop'
>>>>>>>>>> /usr/local/rvm/gems/ruby-2.4.1/gems/passenger-5.1.8/src/ruby_supportlib/phusion_passenger/request_handler.rb:416:in
>>>>>>>>>> `block (3 levels) in start_threads'
>>>>>>>>>> /usr/local/rvm/gems/ruby-2.4.1/gems/passenger-5.1.8/src/ruby_supportlib/phusion_passenger/utils.rb:113:in
>>>>>>>>>> `block in create_thread_and_abort_on_exception'
>>>>>>>>>>
>>>>>>>>>> Craig L Russell
>>>>>>>>>> Secretary, Apache Software Foundation
>>>>>>>>>> c...@apache.org http://db.apache.org/jdo
>>>>>>>>>>
>>>>>>>>
>>>>>>>> Craig L Russell
>>>>>>>> Secretary, Apache Software Foundation
>>>>>>>> c...@apache.org http://db.apache.org/jdo
>>>>>>>
>>>>>>> Craig L Russell
>>>>>>> Secretary, Apache Software Foundation
>>>>>>> c...@apache.org http://db.apache.org/jdo
>>>>>>>
>>>>>
>>>>> Craig L Russell
>>>>> Secretary, Apache Software Foundation
>>>>> c...@apache.org http://db.apache.org/jdo
>>>
>>> Craig L Russell
>>> Secretary, Apache Software Foundation
>>> c...@apache.org http://db.apache.org/jdo
>>
>> Craig L Russell
>> Secretary, Apache Software Foundation
>> c...@apache.org http://db.apache.org/jdo
>>
