Handle leading wildcard in directive path #41
Conversation
|
@dlecocq would you review please and thank you? |
src/agent.cpp
Outdated
| @@ -1,7 +1,6 @@ | |||
| #include <algorithm> | |||
| #include <iomanip> | |||
| #include <sstream> | |||
|
|
|||
There was a problem hiding this comment.
Was this line here to separate system includes from the rest?
There was a problem hiding this comment.
No idea? I probably added or removed it by accident when adding iostream for debugging.
src/agent.cpp
Outdated
| } | ||
|
|
||
| namespace Rep | ||
| { | ||
| Agent& Agent::allow(const std::string& query) | ||
| { | ||
|
|
There was a problem hiding this comment.
I have no explanation for this blank line.
| EXPECT_TRUE(robot.allowed("/get/more/cats/page.html", "meow")); | ||
| EXPECT_TRUE(robot.allowed("/kangaroos/page.html", "meow")); | ||
| EXPECT_TRUE(robot.allowed("/heaps/of/kangaroos/page.html", "meow")); | ||
| EXPECT_TRUE(robot.allowed("/kangaroosandkoalas/page.html", "meow")); |
There was a problem hiding this comment.
I'd like a case with neither cats nor kangaroos.
There was a problem hiding this comment.
The first line has neither cats nor kangaroos:
EXPECT_FALSE(robot.allowed("/kangaroo/zebra/cat/page.html", "meow"));
There was a problem hiding this comment.
Happy to add a specific test if you have one in mind?
| EXPECT_FALSE(robot.allowed("/get/more/cats/page.html", "meow")); | ||
| EXPECT_FALSE(robot.allowed("/kangaroos/page.html", "meow")); | ||
| EXPECT_FALSE(robot.allowed("/heaps/of/kangaroos/page.html", "meow")); | ||
| EXPECT_FALSE(robot.allowed("/kangaroosandkoalas/page.html", "meow")); |
| { | ||
| Url::Url trimmed(trim_front(query, '*')); | ||
| directives_.push_back(Directive(escape_url(trimmed), false)); | ||
| } |
There was a problem hiding this comment.
Did you mean for there to be an else here? As this stands, you're pushing both the trimmed and untrimmed versions.
There was a problem hiding this comment.
Yes, that is the intent. There is an explanation in my PR comment.
|
I know it's already merged, it LGTM! |
This code considers one or more leading wildcard characters (
*) as a special case in theAgentcode. We push an additional directive to thedirectives_vector in theAgent::AllowandAgent::Disallowfunctions which is the result of evaluating the input query stripped of leading '*'s. That is, we evaluate e.g.Disallow: */testasDisallow: /testandDisallow: /*/test.Issue report here: #34