On Sat, Nov 4, 2017 at 6:54 AM, sebb <seb...@gmail.com> wrote:
> 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?
Feels fragile to me: I'm concerned that somebody would go in and edit
the file using a text editor and inadvertently change the line
endings.
My pull request for the mail gem has been accepted, and included a
test case. Once a new release of the mail gem has been made, we can
retire the monkey patch.
- Sam Ruby
>> 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
>>>
