GitHub PullRequestReviewCommentEvent at 2014-02-10 16:23:58
Repository: Haraka
Actor: msimerson
Payload: {"comment":{"url":"https://api.github.com/repos/baudehlo/Haraka/pulls/comments/9589970","id":9589970,"diff_hunk":"@@ -122,61 +82,93 @@ exports.hook_mail = function (next, connection, params) {\n         if (timer) clearTimeout(timer);\n         if (timeout) return;\n         if (err) {\n-            connection.logerror(self, err);\n+            connection.logerror(plugin, err);\n             return next();\n         }\n-        connection.loginfo(self, [\n-            'identity=mfrom',\n-            'ip=' + connection.remote_ip,\n-            'domain=\"' + host + '\"',\n-            'mfrom=<' + mfrom + '>',\n-            'result=' + spf.result(result)\n-            ].join(' '));\n-        // Add a trace header\n-        txn.add_leading_header('Received-SPF', \n-            spf.result(result) +\n-            ' (' + self.config.get('me') + ': domain of ' + host +\n-            ((result === spf.SPF_PASS) ? ' designates ' : ' does not designate ') +\n-            connection.remote_ip + ' as permitted sender) ' + [\n-                'receiver=' + self.config.get('me'),\n-                'identity=mailfrom',\n-                'client-ip=' + connection.remote_ip,\n-                'helo=' + connection.hello_host,\n-                'envelope-from=<' + mfrom + '>',\n-            ].join('; '));\n-        auth_result = spf.result(result).toLowerCase();\n+        plugin.log_result(connection, 'mfrom', host, mfrom, spf.result(result));\n+        plugin.save_to_header(connection, spf, result, mfrom, host, 'mailfrom');\n+\n+        var auth_result = spf.result(result).toLowerCase();\n         connection.auth_results( \"spf=\"+auth_result+\" smtp.mailfrom=\"+host);\n+\n         txn.notes.spf_mail_result = spf.result(result);\n         txn.notes.spf_mail_record = spf.spf_record;\n-        switch (result) {\n-            case spf.SPF_NONE:\n-            case spf.SPF_NEUTRAL:\n-            case spf.SPF_PASS:\n-                return next();\n-            case spf.SPF_SOFTFAIL:\n-                if (cfg.main.mail_softfail_reject) {\n-                    return next(DENY, 'sender <' + mfrom + '> SPF SoftFail');\n-                }\n-                return next();\n-            case spf.SPF_FAIL:\n-                if (cfg.main.mail_fail_reject) {\n-                    return next(DENY, 'sender <' + mfrom + '> SPF Fail');\n-                }\n-                return next();\n-            case spf.SPF_TEMPERROR:\n-                if (cfg.main.mail_temperror_defer) {\n-                    return next(DENYSOFT, 'sender <' + mfrom + '> SPF Temporary Error');\n-                }\n-                return next();\n-            case spf.SPF_PERMERROR:\n-                if (cfg.main.mail_permerror_reject) {\n-                    return next(DENY, 'sender <' + mfrom + '> SPF Permanent Error');\n-                }\n-                return next();\n-            default:\n-                // Unknown result\n-                connection.logerror(self, 'unknown result code=' + result);\n-                return next();\n+        var spf_note = {\n+            scope: 'mfrom',\n+            result: spf.result(result),\n+            domain: host,\n+        };\n+        if (connection.transaction) {","path":"plugins/spf.js","position":211,"original_position":211,"commit_id":"6fe7c22d4542c152922c1583bb4a33446f4550d5","original_commit_id":"6fe7c22d4542c152922c1583bb4a33446f4550d5","user":{"login":"msimerson","id":261635,"avatar_url":"https://gravatar.com/avatar/f65de8c89a9acee50957c9cd09811eb8?d=https%3A%2F%2Fidenticons.github.com%2Ff8385ee05b21b6374382d329241586e4.png&r=x","gravatar_id":"f65de8c89a9acee50957c9cd09811eb8","url":"https://api.github.com/users/msimerson","html_url":"https://github.com/msimerson","followers_url":"https://api.github.com/users/msimerson/followers","following_url":"https://api.github.com/users/msimerson/following{/other_user}","gists_url":"https://api.github.com/users/msimerson/gists{/gist_id}","starred_url":"https://api.github.com/users/msimerson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/msimerson/subscriptions","organizations_url":"https://api.github.com/users/msimerson/orgs","repos_url":"https://api.github.com/users/msimerson/repos","events_url":"https://api.github.com/users/msimerson/events{/privacy}","received_events_url":"https://api.github.com/users/msimerson/received_events","type":"User","site_admin":false},"body":"I added that after Haraka went down because at some point, the transaction wasn't available when I tried to save the note. My guess was a timing issue. Maybe another data_post issued a deny. Irregardless of why, it hasn't crashed again since.\r\n\r\nMatt\r\n\r\n\r\n\r\n> On Feb 10, 2014, at 1:20 AM, Steve Freegard <notifications@github.com> wrote:\r\n> \r\n> In plugins/spf.js:\r\n> \r\n> > -                return next();\r\n> > -            case spf.SPF_PERMERROR:\r\n> > -                if (cfg.main.mail_permerror_reject) {\r\n> > -                    return next(DENY, 'sender <' + mfrom + '> SPF Permanent Error');\r\n> > -                }\r\n> > -                return next();\r\n> > -            default:\r\n> > -                // Unknown result\r\n> > -                connection.logerror(self, 'unknown result code=' + result);\r\n> > -                return next();\r\n> > +        var spf_note = {\r\n> > +            scope: 'mfrom',\r\n> > +            result: spf.result(result),\r\n> > +            domain: host,\r\n> > +        };\r\n> > +        if (connection.transaction) {\r\n> This block seems to be redundant - we're running on hook_mail and therefore we should always be recording the note to connection.transaction.notes and not to connection.notes.\r\n> \r\n> —\r\n> Reply to this email directly or view it on GitHub.","created_at":"2014-02-10T16:23:58Z","updated_at":"2014-02-10T16:23:58Z","html_url":"https://github.com/baudehlo/Haraka/pull/452#discussion_r9589970","pull_request_url":"https://api.github.com/repos/baudehlo/Haraka/pulls/452","_links":{"self":{"href":"https://api.github.com/repos/baudehlo/Haraka/pulls/comments/9589970"},"html":{"href":"https://github.com/baudehlo/Haraka/pull/452#discussion_r9589970"},"pull_request":{"href":"https://api.github.com/repos/baudehlo/Haraka/pulls/452"}}}}