search

trailofbits / spf-query

Ruby SPF Parser
MIT License
29 stars 15 forks source link

Parser should allow %{v} macro_string #14

Closed rfgil closed 1 year ago

rfgil commented 1 year ago

Hi! Thank you for this gem.

I noticed the gem is failing to parse the following SPF Record: v=spf1 include:%{ir}.%{v}.%{d}.spf.has.pphosted.com -all:

DEVELOPMENT irb(main):001> SPF::Query::Parser.parse("v=spf1 include:%{ir}.%{v}.%{d}.spf.has.pphosted.com -all")
/Users/rafael/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/parslet-1.8.2/lib/parslet/cause.rb:70:in `raise': Failed to match sequence (VERSION SP{1, } rules:TERMS SP{0, }) at line 1 char 22. (Parslet::ParseFailed)

This is because the parser is not expecting the %{v} macro, which is valid according to the documentation: https://datatracker.ietf.org/doc/html/rfc7208#section-7.4.

Thus, this PR adds the missing v as a valid macro_letter.

woodruffw commented 1 year ago

Thanks @rfgil! This looks reasonable to me.

woodruffw commented 1 year ago

Would you mind adding some tests as well?

rfgil commented 1 year ago

Would you mind adding some tests as well?

@woodruffw Sure, just added a spec with the SPF record above. Let me know if it's as you expected. Thanks!

woodruffw commented 1 year ago

Yep, LGTM. Thanks @rfgil!