Skip to content

Fix formatting issue #17

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

Closed
wants to merge 1 commit into from
Closed

Conversation

HugoDelval
Copy link

@HugoDelval HugoDelval commented Dec 19, 2017

There was an issue here when the Rule was too long (example: HEADER_FROM_DIFFERENT_DOMAINS) then 28-len(line) was negative. This caused this line to issue a runtime error. (Happy to discuss this if needed)

@HugoDelval HugoDelval requested a review from arp242 as a code owner December 19, 2017 16:45
@arp242
Copy link

arp242 commented Dec 19, 2017

Could you please remove the changes to Report.String and make a separate pull request for that? That will make it easier to review the changes and write tests for them.

I'll take a look at the connection changes later this evening. Thanks! 👍

@HugoDelval HugoDelval changed the title Ability to use Unix socket + fix formatting issue Fix formatting issue Dec 19, 2017
@HugoDelval
Copy link
Author

Good idea. Done :)
(I've edited this PR and added a new one)

@HugoDelval HugoDelval force-pushed the master branch 2 times, most recently from e605e00 to 209c443 Compare December 19, 2017 17:29
@arp242
Copy link

arp242 commented Dec 20, 2017

What do you think of this solution? This makes sure it works for any length. It's also how SpamAssassin prints long rule names.

diff --git i/spamc.go w/spamc.go
index d51ef44..0fbcec9 100644
--- i/spamc.go
+++ w/spamc.go
@@ -359,7 +359,12 @@ func (r Report) String() string {
                }

                line := fmt.Sprintf("%v%.1f %v", leadingSpace, t.Points, t.Rule)
-               line += strings.Repeat(" ", 28-len(line)) + t.Description + "\n"
+               nspaces := 27 - len(line)
+               spaces := " "
+               if nspaces > 0 {
+                       spaces += strings.Repeat(" ", nspaces)
+               }
+               line += spaces + t.Description + "\n"
                table += line
        }

diff --git i/spamc_test.go w/spamc_test.go
index d2418f2..44ed606 100644
--- i/spamc_test.go
+++ w/spamc_test.go
@@ -267,6 +267,7 @@ func TestParseReport(t *testing.T) {
                                 0.4 INVALID_DATE           Invalid Date: header (not RFC 2822)
                                -0.0 NO_RELAYS              Informational: message was not relayed via SMTP
                                -1.2 MISSING_HEADERS        Missing To: header
+                                0.1 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail are different
                        `),
                        Report{
                                Intro: normalizeSpace(`
@@ -297,6 +298,11 @@ func TestParseReport(t *testing.T) {
                                                Rule:        "MISSING_HEADERS",
                                                Description: "Missing To: header",
                                        },
+                                       {
+                                               Points:      0.1,
+                                               Rule:        "HEADER_FROM_DIFFERENT_DOMAINS",
+                                               Description: "From and EnvelopeFrom 2nd level mail are different",
+                                       },
                                },
                        },
                },

@arp242
Copy link

arp242 commented Dec 20, 2017

I fixed it with the above patch in #20. Thanks!

@arp242 arp242 closed this Dec 20, 2017
@HugoDelval
Copy link
Author

Looks great ! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants