[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: [kdelibs] kioslave/http/kcookiejar: Initialise the mCrossDomain member variable in the cookies
From:       Dawit A <adawit () kde ! org>
Date:       2013-01-19 16:22:24
Message-ID: CALa28R6p=fMjcV7uq4BQ858YVD1ykuvKcd+YbyP=R3Grf3u8dQ () mail ! gmail ! com
[Download RAW message or body]

On Fri, Jan 18, 2013 at 6:22 AM, Thiago Macieira
<thiago.macieira@intel.com>wrote:

> Git commit 794b14b8af5b610fc3eed6945f93f0c69dd49a9a by Thiago Macieira.
> Committed on 18/01/2013 at 12:12.
> Pushed by thiago into branch 'master'.
>
> Initialise the mCrossDomain member variable in the cookies
>
> For several months now, all my cookies would be forgotten after a kded
> restart. After debugging the problem, turns out that mCrossDomain was
> of value 127, which makes no sense for a boolean.
>

Good catch. This is something the extensive cookiejar test cases did not
account for because it requires saving cookies and restarting the
cookiejar. The test cases never anticipated that ; so I will have to add a
test case for this and backport the fix into the 4.10 branch.


> This variable has been present since 2002, which means that the
> "reject cross domain cookies" feature has been broken for 10 years and
> 8 months.
>

Actually that statement is not correct. The "reject cross domain" cookies
functionality is not affected by this bug at all. What is affected is far
worse and only happens if kded (and hence kcookiejar) is shutdown and
restarted. When the cookiejar restarts it reloads your saved cookies from a
file. Unfortunately it does not explicitly initialize the cross domain flag
to false. As a result, all of your saved cookies will be loaded with their
cross domain flag set arbitrarily. In your case it seems to be 127. That
means none of your saved cookies will work (they will be rejected as cross
domain) and you won't be automatically logged into the sites. Depending on
your cookie policy settings that is either the correct thing or a bug as is
the case here.

The problem is further compounded by the fact that not everyone sees this
issue. On my system (gcc 4.7.2), uninitialized boolean variable seems to
always be set to false even when compiled with the optimization on. Hence,
I have never been able to reproduce the regression this mistake causes (see
https://bugs.kde.org/show_bug.cgi?id=307832).

Anyhow, thanks for debugging and fixing this.

[Attachment #3 (text/html)]

<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, \
Jan 18, 2013 at 6:22 AM, Thiago Macieira <span dir="ltr">&lt;<a \
href="mailto:thiago.macieira@intel.com" \
target="_blank">thiago.macieira@intel.com</a>&gt;</span> wrote:<br>


<blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Git \
commit 794b14b8af5b610fc3eed6945f93f0c69dd49a9a by Thiago Macieira.<br>


Committed on 18/01/2013 at 12:12.<br>
Pushed by thiago into branch &#39;master&#39;.<br>
<br>
Initialise the mCrossDomain member variable in the cookies<br>
<br>
For several months now, all my cookies would be forgotten after a kded<br>
restart. After debugging the problem, turns out that mCrossDomain was<br>
of value 127, which makes no sense for a \
boolean.<br></blockquote><div><br></div><div>Good catch. This is something the \
extensive cookiejar test cases did not account for because it requires saving cookies \
and restarting the cookiejar. The test cases never anticipated that ; so I will have \
to add a test case for this and backport the fix into the 4.10 branch.</div>


<div>  </div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
 This variable has been present since 2002, which means that the<br>
&quot;reject cross domain cookies&quot; feature has been broken for 10 years and<br>
8 months.<br></blockquote><div><br></div><div>Actually that statement is not correct. \
The &quot;reject cross domain&quot; cookies functionality is not affected by this bug \
at all. What is affected is far worse and only happens if kded (and hence kcookiejar) \
is shutdown and restarted. When the cookiejar restarts it reloads your saved cookies \
from a file. Unfortunately it does not explicitly initialize the cross domain flag to \
false. As a result, all of your saved cookies will be loaded with their cross domain \
flag set arbitrarily. In your case it seems to be 127. That means none of your saved \
cookies will work (they will be rejected as cross domain) and you won&#39;t be \
automatically logged into the sites. Depending on your cookie policy settings that is \
either the correct thing or a bug as is the case here.</div>


<div><br></div><div>The problem is further compounded by the fact that not everyone \
sees this issue. On my system (gcc 4.7.2), uninitialized boolean variable seems to \
always be set to false even when compiled with the optimization on. Hence, I have \
never been able to reproduce the regression this mistake causes (see  <a \
href="https://bugs.kde.org/show_bug.cgi?id=307832">https://bugs.kde.org/show_bug.cgi?id=307832</a>).</div>


<div><br></div><div>Anyhow, thanks for debugging and fixing this.</div>
</div></div></div>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic