Re: [freenet-dev] Problem with handling of Opennet Announce Requests

2016-02-23 Thread Matthew Toseland
On 22/02/16 03:49, Steve Dougherty wrote:
> On 02/18/2016 09:06 AM, Matthew Toseland wrote:
>> On 18/02/16 13:43, Steve Dougherty wrote:
>>> On Thu, Feb 18, 2016, 6:30 AM Martin Byrenheid <
>>> martin.byrenh...@tu-dresden.de> wrote:
>>>
 Hi,

 while working with Freenet, I discovered that whenever a seed node
 received an
 OpennetAnnounceRequest-message for a target location X, it forwards the
 request to another opennet peer node, but always for target location 0.0
 instead.
>>> Yikes. That sure sounds like a bug.
>>>
>>> This behavior results from the fact that the constructor of the
 AnnounceSender class (line 55 [1])  does not copy the given target location
 into the "target" member variable. Is this an implementation bug or is
 there a
 good reason why the original target location should be ignored?

>>> Matthew likely wrote it, but he's busy at university, so instead of asking
>>> him my impulse would be to check if that behavior is in the initial
>>> AnnounceSender implementation. If it was intentionally removed later there
>>> should be reasoning in the commit message. If - as I'd expect - it's a bug,
>>> it very well may exist in the initial implementation or be introduced by a
>>> refactor.
>> Argh. Yup:
>>
>> https://github.com/freenet/fred/commit/5d21c855655c1f974a8e9333d74c1d564224bf4c
>>
>> Please send a patch, and make target final while you're at it. Thanks!
> Patch is now merged as 71a788164a896d6e5b6af0dfe0f35da5a6927633. Thanks
> again!
>
> Add missing assignment for Opennet announce location
>
> This regression was introduced in
> 5d21c855655c1f974a8e9333d74c1d564224bf4c which was released in
> build01412 on 2012-09-12.
>
> See GitHub for details:
> https://github.com/freenet/fred/pull/503#issuecomment-186988870
>
> - Steve

Woot!

I appreciate that we don't want to delay 1471 further, but I would like
to point out that the above is a major bugfix, potentially a significant
performance gain for bootstrapping new nodes on opennet. Which in turn
affects retention of new users trying Freenet, and thus developers and
funding. So please try to get it out reasonably soon after 1471.



signature.asc
Description: OpenPGP digital signature
___
Devl mailing list
Devl@freenetproject.org
https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl

Re: [freenet-dev] Problem with handling of Opennet Announce Requests

2016-02-21 Thread Steve Dougherty
On 02/18/2016 09:06 AM, Matthew Toseland wrote:
> On 18/02/16 13:43, Steve Dougherty wrote:
>> On Thu, Feb 18, 2016, 6:30 AM Martin Byrenheid <
>> martin.byrenh...@tu-dresden.de> wrote:
>>
>>> Hi,
>>>
>>> while working with Freenet, I discovered that whenever a seed node
>>> received an
>>> OpennetAnnounceRequest-message for a target location X, it forwards the
>>> request to another opennet peer node, but always for target location 0.0
>>> instead.
>>
>> Yikes. That sure sounds like a bug.
>>
>> This behavior results from the fact that the constructor of the
>>> AnnounceSender class (line 55 [1])  does not copy the given target location
>>> into the "target" member variable. Is this an implementation bug or is
>>> there a
>>> good reason why the original target location should be ignored?
>>>
>> Matthew likely wrote it, but he's busy at university, so instead of asking
>> him my impulse would be to check if that behavior is in the initial
>> AnnounceSender implementation. If it was intentionally removed later there
>> should be reasoning in the commit message. If - as I'd expect - it's a bug,
>> it very well may exist in the initial implementation or be introduced by a
>> refactor.
> 
> Argh. Yup:
> 
> https://github.com/freenet/fred/commit/5d21c855655c1f974a8e9333d74c1d564224bf4c
> 
> Please send a patch, and make target final while you're at it. Thanks!

Patch is now merged as 71a788164a896d6e5b6af0dfe0f35da5a6927633. Thanks
again!

Add missing assignment for Opennet announce location

This regression was introduced in
5d21c855655c1f974a8e9333d74c1d564224bf4c which was released in
build01412 on 2012-09-12.

See GitHub for details:
https://github.com/freenet/fred/pull/503#issuecomment-186988870

- Steve



signature.asc
Description: OpenPGP digital signature
___
Devl mailing list
Devl@freenetproject.org
https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl

Re: [freenet-dev] Problem with handling of Opennet Announce Requests

2016-02-18 Thread Matthew Toseland
On 18/02/16 13:43, Steve Dougherty wrote:
> On Thu, Feb 18, 2016, 6:30 AM Martin Byrenheid <
> martin.byrenh...@tu-dresden.de> wrote:
>
>> Hi,
>>
>> while working with Freenet, I discovered that whenever a seed node
>> received an
>> OpennetAnnounceRequest-message for a target location X, it forwards the
>> request to another opennet peer node, but always for target location 0.0
>> instead.
>
> Yikes. That sure sounds like a bug.
>
> This behavior results from the fact that the constructor of the
>> AnnounceSender class (line 55 [1])  does not copy the given target location
>> into the "target" member variable. Is this an implementation bug or is
>> there a
>> good reason why the original target location should be ignored?
>>
> Matthew likely wrote it, but he's busy at university, so instead of asking
> him my impulse would be to check if that behavior is in the initial
> AnnounceSender implementation. If it was intentionally removed later there
> should be reasoning in the commit message. If - as I'd expect - it's a bug,
> it very well may exist in the initial implementation or be introduced by a
> refactor.

Argh. Yup:

https://github.com/freenet/fred/commit/5d21c855655c1f974a8e9333d74c1d564224bf4c

Please send a patch, and make target final while you're at it. Thanks!



signature.asc
Description: OpenPGP digital signature
___
Devl mailing list
Devl@freenetproject.org
https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl

Re: [freenet-dev] Problem with handling of Opennet Announce Requests

2016-02-18 Thread Steve Dougherty
On Thu, Feb 18, 2016, 6:30 AM Martin Byrenheid <
martin.byrenh...@tu-dresden.de> wrote:

> Hi,
>
> while working with Freenet, I discovered that whenever a seed node
> received an
> OpennetAnnounceRequest-message for a target location X, it forwards the
> request to another opennet peer node, but always for target location 0.0
> instead.


Yikes. That sure sounds like a bug.

This behavior results from the fact that the constructor of the
> AnnounceSender class (line 55 [1])  does not copy the given target location
> into the "target" member variable. Is this an implementation bug or is
> there a
> good reason why the original target location should be ignored?
>

Matthew likely wrote it, but he's busy at university, so instead of asking
him my impulse would be to check if that behavior is in the initial
AnnounceSender implementation. If it was intentionally removed later there
should be reasoning in the commit message. If - as I'd expect - it's a bug,
it very well may exist in the initial implementation or be introduced by a
refactor.

Thanks,
Steve

Martin
>
> [1]
>
> https://github.com/freenet/fred/blob/2e89c8620413bdf04e20268ba7afbad05d4c8b6f/src/freenet/node/AnnounceSender.java#L55
> ___
> Devl mailing list
> Devl@freenetproject.org
> https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
___
Devl mailing list
Devl@freenetproject.org
https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl

[freenet-dev] Problem with handling of Opennet Announce Requests

2016-02-18 Thread Martin Byrenheid
Hi,

while working with Freenet, I discovered that whenever a seed node received an 
OpennetAnnounceRequest-message for a target location X, it forwards the 
request to another opennet peer node, but always for target location 0.0 
instead.  This behavior results from the fact that the constructor of the 
AnnounceSender class (line 55 [1])  does not copy the given target location 
into the "target" member variable. Is this an implementation bug or is there a 
good reason why the original target location should be ignored?

Martin

[1] 
https://github.com/freenet/fred/blob/2e89c8620413bdf04e20268ba7afbad05d4c8b6f/src/freenet/node/AnnounceSender.java#L55
___
Devl mailing list
Devl@freenetproject.org
https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl

[freenet-dev] Problem with handling of Opennet Announce Requests

2016-02-18 Thread Martin Byrenheid
Hi,

while working with Freenet, I discovered that whenever a seed node received an 
OpennetAnnounceRequest-message for a target location X, it forwards the 
request to another opennet peer node, but always for target location 0.0 
instead.  This behavior results from the fact that the constructor of the 
AnnounceSender class (line 55 [1])  does not copy the given target location 
into the "target" member variable. Is this an implementation bug or is there a 
good reason why the original target location should be ignored?

Martin

[1] 
https://github.com/freenet/fred/blob/2e89c8620413bdf04e20268ba7afbad05d4c8b6f/src/freenet/node/AnnounceSender.java#L55
___
Devl mailing list
Devl@freenetproject.org
https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl