New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cannot send email with only bcc #14

Closed
mmerickel opened this Issue Mar 15, 2012 · 16 comments

Comments

Projects
None yet
3 participants
@mmerickel
Member

mmerickel commented Mar 15, 2012

message = Message(subject='test', body='this is a test', bcc=['me@example.com'])
mailer.send(message)
transaction.commit()
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "test.py", line 83, in <module>
    main(sys.argv)
  File "test.py", line 79, in main
    mailer.send(message)
  File "lib/python2.7/site-packages/pyramid_mailer/mailer.py", line 178, in send
    return self.direct_delivery.send(*self._message_args(message))
  File "lib/python2.7/site-packages/pyramid_mailer/mailer.py", line 222, in _message_args
    message.to_message())
  File "lib/python2.7/site-packages/pyramid_mailer/message.py", line 82, in to_message
    self.validate()
  File "lib/python2.7/site-packages/pyramid_mailer/message.py", line 132, in validate
    raise InvalidMessage, "No recipients have been added"
pyramid_mailer.exceptions.InvalidMessage: No recipients have been added
@rpatterson

This comment has been minimized.

Show comment
Hide comment
@rpatterson

rpatterson Mar 15, 2012

Contributor

See #10 for possibly related history. I'm wondering, as @mcdonc did, if that pull request introduced this bug.

Contributor

rpatterson commented Mar 15, 2012

See #10 for possibly related history. I'm wondering, as @mcdonc did, if that pull request introduced this bug.

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Mar 15, 2012

Member

Note that Message(cc=..) has a similar problem. All scenarios require the recipients argument for some reason.

Member

mmerickel commented Mar 15, 2012

Note that Message(cc=..) has a similar problem. All scenarios require the recipients argument for some reason.

rpatterson added a commit to rpatterson/pyramid_mailer that referenced this issue Mar 15, 2012

@mcdonc

This comment has been minimized.

Show comment
Hide comment
@mcdonc

mcdonc Mar 20, 2012

Member

The tests for cc-only and bcc-only worked under Python 2.6 but failed under Python 3.2 on rpatterson's master. Instead of fixing it, I just caused pyramid_mailer to raise a more descriptive error message saying:

            raise InvalidMessage("Must have at least one direct recipient "
                                 "even if cc or bcc set")

We can revisit this later, I just couldn't figure out how to make it work under 3.2.

Member

mcdonc commented Mar 20, 2012

The tests for cc-only and bcc-only worked under Python 2.6 but failed under Python 3.2 on rpatterson's master. Instead of fixing it, I just caused pyramid_mailer to raise a more descriptive error message saying:

            raise InvalidMessage("Must have at least one direct recipient "
                                 "even if cc or bcc set")

We can revisit this later, I just couldn't figure out how to make it work under 3.2.

@rpatterson

This comment has been minimized.

Show comment
Hide comment
@rpatterson

rpatterson Mar 20, 2012

Contributor

Odd, I re-ran tox before pulling and merging and the tests passed under 3.2 for me. Investigating now.

Contributor

rpatterson commented Mar 20, 2012

Odd, I re-ran tox before pulling and merging and the tests passed under 3.2 for me. Investigating now.

@rpatterson

This comment has been minimized.

Show comment
Hide comment
@rpatterson

rpatterson Mar 20, 2012

Contributor

When I run tox against 95ff3e7, your first commit on top of my master, I see the failures in all envs. When I run tox against d2c6adf, my last change I see only an unrelated failure under jython (which I missed before, sorry). So I'm not seeing a 3.2 specific failure.

But reading your comment and commit logs more carefully, it seems like you want pyramid_mailer to enforce a prohibition against cc/bcc only messages so I guess I just shouldn't have gone off and implemented what mmerickel requested without discussion. Oops, sorry. :-)

Contributor

rpatterson commented Mar 20, 2012

When I run tox against 95ff3e7, your first commit on top of my master, I see the failures in all envs. When I run tox against d2c6adf, my last change I see only an unrelated failure under jython (which I missed before, sorry). So I'm not seeing a 3.2 specific failure.

But reading your comment and commit logs more carefully, it seems like you want pyramid_mailer to enforce a prohibition against cc/bcc only messages so I guess I just shouldn't have gone off and implemented what mmerickel requested without discussion. Oops, sorry. :-)

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Mar 20, 2012

Member

@rpatterson, he doesn't want to enforce against cc/bcc only messages. That was just his clear error message until he could revisit the issue. Keep fighting. :-)

Member

mmerickel commented Mar 20, 2012

@rpatterson, he doesn't want to enforce against cc/bcc only messages. That was just his clear error message until he could revisit the issue. Keep fighting. :-)

@rpatterson

This comment has been minimized.

Show comment
Hide comment
@rpatterson

rpatterson Mar 20, 2012

Contributor

@mmerickel You keep fighting! And here's how... :-)

Checkout d2c6adf and see if the 3.2 tests pass for you.

Contributor

rpatterson commented Mar 20, 2012

@mmerickel You keep fighting! And here's how... :-)

Checkout d2c6adf and see if the 3.2 tests pass for you.

@rpatterson

This comment has been minimized.

Show comment
Hide comment
@rpatterson

rpatterson Mar 20, 2012

Contributor

@mmerickel Are you sure he doesn't want to? Have you looked at 95ff3e7? @mcdonc Care to clarify whether you want to allow messages with cc/bcc but no recipients?

Contributor

rpatterson commented Mar 20, 2012

@mmerickel Are you sure he doesn't want to? Have you looked at 95ff3e7? @mcdonc Care to clarify whether you want to allow messages with cc/bcc but no recipients?

@mcdonc

This comment has been minimized.

Show comment
Hide comment
@mcdonc

mcdonc Mar 20, 2012

Member

No I don't want to enforce against it. I just couldn't figure out the failure I see.

When I comment out the check I added in message.py:

        ## if (self.cc or self.bcc) and not self.recipients:
        ##     raise InvalidMessage("Must have at least one direct recipient "
        ##                          "even if cc or bcc set")

And add this test to tests.TestMessage

    def test_cc_without_recipients_2(self):

        from pyramid_mailer.message import Message

        msg = Message(subject="testing",
                      sender="sender@example.com",
                      body="testing",
                      cc=["tosomeoneelse@example.com"])
        response = msg.get_response()
        self.assertTrue("Cc: tosomeoneelse@example.com" in str(response))

This is the exception I get on my system under Python 3.2:


======================================================================
ERROR: test_cc_without_recipients_2 (pyramid_mailer.tests.TestMessage)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/chrism/projects/pyramid_mailer/pyramid_mailer/tests.py", line 173, in test_cc_without_recipients
    self.assertTrue("Cc: tosomeoneelse@example.com" in str(response))
  File "/home/chrism/projects/pyramid_mailer/pyramid_mailer/response.py", line 239, in __str__
    return self.to_message().as_string()
  File "/home/chrism/opt/Python-3.2/lib/python3.2/email/message.py", line 167, in as_string
    g.flatten(self, unixfrom=unixfrom)
  File "/home/chrism/opt/Python-3.2/lib/python3.2/email/generator.py", line 88, in flatten
    self._write(msg)
  File "/home/chrism/opt/Python-3.2/lib/python3.2/email/generator.py", line 141, in _write
    self._write_headers(msg)
  File "/home/chrism/opt/Python-3.2/lib/python3.2/email/generator.py", line 176, in _write_headers
    self.write(header.encode(linesep=self._NL)+self._NL)
  File "/home/chrism/opt/Python-3.2/lib/python3.2/email/header.py", line 317, in encode
    formatter.feed(lines[0], charset)
IndexError: list index out of range

It doesn't happen on Python 2.6 or 2.7. I noticed while PDB stepping through it that the "To" header is the empty string. I suspect that may be why but I didn't chase it down and punted by disallowing it.

Member

mcdonc commented Mar 20, 2012

No I don't want to enforce against it. I just couldn't figure out the failure I see.

When I comment out the check I added in message.py:

        ## if (self.cc or self.bcc) and not self.recipients:
        ##     raise InvalidMessage("Must have at least one direct recipient "
        ##                          "even if cc or bcc set")

And add this test to tests.TestMessage

    def test_cc_without_recipients_2(self):

        from pyramid_mailer.message import Message

        msg = Message(subject="testing",
                      sender="sender@example.com",
                      body="testing",
                      cc=["tosomeoneelse@example.com"])
        response = msg.get_response()
        self.assertTrue("Cc: tosomeoneelse@example.com" in str(response))

This is the exception I get on my system under Python 3.2:


======================================================================
ERROR: test_cc_without_recipients_2 (pyramid_mailer.tests.TestMessage)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/chrism/projects/pyramid_mailer/pyramid_mailer/tests.py", line 173, in test_cc_without_recipients
    self.assertTrue("Cc: tosomeoneelse@example.com" in str(response))
  File "/home/chrism/projects/pyramid_mailer/pyramid_mailer/response.py", line 239, in __str__
    return self.to_message().as_string()
  File "/home/chrism/opt/Python-3.2/lib/python3.2/email/message.py", line 167, in as_string
    g.flatten(self, unixfrom=unixfrom)
  File "/home/chrism/opt/Python-3.2/lib/python3.2/email/generator.py", line 88, in flatten
    self._write(msg)
  File "/home/chrism/opt/Python-3.2/lib/python3.2/email/generator.py", line 141, in _write
    self._write_headers(msg)
  File "/home/chrism/opt/Python-3.2/lib/python3.2/email/generator.py", line 176, in _write_headers
    self.write(header.encode(linesep=self._NL)+self._NL)
  File "/home/chrism/opt/Python-3.2/lib/python3.2/email/header.py", line 317, in encode
    formatter.feed(lines[0], charset)
IndexError: list index out of range

It doesn't happen on Python 2.6 or 2.7. I noticed while PDB stepping through it that the "To" header is the empty string. I suspect that may be why but I didn't chase it down and punted by disallowing it.

@rpatterson

This comment has been minimized.

Show comment
Hide comment
@rpatterson

rpatterson Mar 20, 2012

Contributor

@mcdonc Ok, I'll apply that patch and investigate. Thanks!

Contributor

rpatterson commented Mar 20, 2012

@mcdonc Ok, I'll apply that patch and investigate. Thanks!

@mcdonc

This comment has been minimized.

Show comment
Hide comment
@mcdonc

mcdonc Mar 20, 2012

Member

By the way I made new releases of pyramid_mailer and repoze.sendmail last night, thanks for the work Ross!

Member

mcdonc commented Mar 20, 2012

By the way I made new releases of pyramid_mailer and repoze.sendmail last night, thanks for the work Ross!

@rpatterson

This comment has been minimized.

Show comment
Hide comment
@rpatterson

rpatterson Mar 20, 2012

Contributor

@mcdonc Yeah, I don't get that error or any failure under 3.2 when I revert your related changes and then add the test you provided above.

@mmerickel Can you try running tox against rpatterson/pyramid_mailer@fb2a013?

Contributor

rpatterson commented Mar 20, 2012

@mcdonc Yeah, I don't get that error or any failure under 3.2 when I revert your related changes and then add the test you provided above.

@mmerickel Can you try running tox against rpatterson/pyramid_mailer@fb2a013?

@mcdonc

This comment has been minimized.

Show comment
Hide comment
@mcdonc

mcdonc Mar 20, 2012

Member

I'm on Python 3.2 (not 3.2.1 or 3.2.2), maybe that's the difference.

Member

mcdonc commented Mar 20, 2012

I'm on Python 3.2 (not 3.2.1 or 3.2.2), maybe that's the difference.

@rpatterson

This comment has been minimized.

Show comment
Hide comment
@rpatterson

rpatterson Mar 20, 2012

Contributor

Ok, I'm testing 3.2 now

Contributor

rpatterson commented Mar 20, 2012

Ok, I'm testing 3.2 now

@rpatterson

This comment has been minimized.

Show comment
Hide comment
@rpatterson

rpatterson Mar 20, 2012

Contributor

Hmm, I couldn't build build 3.2 with ssl support, I think because of http://bugs.python.org/issue12012. Without ssl, the imports in the tests are broken. So I manually applied the patch that fixes that bug:

http://hg.python.org/cpython/raw-rev/20beec22764f

Then I was able to reproduce the issue and commit a workaround that skips headers that would be empty strings which seems appropriate anyways.

Contributor

rpatterson commented Mar 20, 2012

Hmm, I couldn't build build 3.2 with ssl support, I think because of http://bugs.python.org/issue12012. Without ssl, the imports in the tests are broken. So I manually applied the patch that fixes that bug:

http://hg.python.org/cpython/raw-rev/20beec22764f

Then I was able to reproduce the issue and commit a workaround that skips headers that would be empty strings which seems appropriate anyways.

@mcdonc

This comment has been minimized.

Show comment
Hide comment
@mcdonc

mcdonc Mar 21, 2012

Member

This was fixed with the merge of the above-mentioned pull request. Thanks Ross!

Member

mcdonc commented Mar 21, 2012

This was fixed with the merge of the above-mentioned pull request. Thanks Ross!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment