[Webkit-unassigned] [Bug 117497] Improve algorithm of removing user and pass, by seperating new function

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=117497


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #205554|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #18 from Darin Adler <darin at apple.com>  2013-08-27 09:31:44 PST ---
(From update of attachment 205554)
View in context: https://bugs.webkit.org/attachment.cgi?id=205554&action=review

Definitely don’t want to land this with the extra m_userStart == end check in it. But also, I want some reassurance that we have test coverage for the code that is looking for a "@" character. And a comment explaining why it's there.

> Source/WebCore/platform/KURL.cpp:808
>  }
> +bool KURL::removeUserAndPassword()

Need a blank line between functions.

> Source/WebCore/platform/KURL.cpp:818
> +    if (m_userStart == end)
> +        return false;
> +
> +    if (m_userStart != end && end != m_hostEnd && m_string[end] == '@')
> +        end += 1;

This checks m_userStart == end twice in a row.

The end += 1 code is too confusing without a why comment. It seems like we copied it from setPass, but removed the comment and explanation.

I also have no idea why the end += 1 is needed. I doubt that there can be a "@" character *after* m_passwordEnd. Is that really something that happens in our parsing? Is there a test case that covers this?

> Source/WebCore/platform/KURL.cpp:822
> +}
>  void KURL::setFragmentIdentifier(const String& s)

Need a blank line between functions.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.