I would also add logging to the code that build these headers to figure out
when a negative length occurs. Anyway, I will let Yuri handle this one :-).

Lennard
On Jan 9, 2012 1:48 AM, "Yuri Zelikov" <[email protected]> wrote:

>    This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3432/
>
> Seems like there's another code review about the same issue - 
> https://reviews.apache.org/r/3431/.
>
>
>
> /src/org/waveprotocol/box/server/persistence/file/FileDeltaCollection.java<https://reviews.apache.org/r/3432/diff/1/?file=67165#file67165line512>
>  (Diff
> revision 1)
>
> private int writeAppliedDelta(ByteStringMessage<ProtocolAppliedWaveletDelta> 
> delta)
>
>    512
>
>     }
>
>   IMO, it's not correct to substitute negative number with zero here. 
> FileDeltaCollection should fail in this case and let the caller deal with the 
> consequences, i.e. WaveMap.
>
>
> - Yuri
>
> On January 8th, 2012, 11:28 p.m., Ali Lown wrote:
>   Review request for wave, Yuri Zelikov and Lennard de Rijk.
> By Ali Lown.
>
> *Updated 2012-01-08 23:28:50*
> Description
>
> Nothing exciting in this patch, just prevents the length being processed as 
> <0 (causing a whole server crash) for an uncaught exception.
>
> This lets the server boot (which is much more important that a few corrupt 
> deltas), and the rest of the persistence layer still leaves a SEVERE in my 
> logs to tell me about the corrupt delta in the wave.
>
> As for how it got corrupt I don't know so I can't address that actual issue.
>
>   Testing
>
> I wrote, compiled and deployed. Then my server booted again.
>
>   *Bugs: * WAVE-321 <https://issues.apache.org/jira/browse/WAVE-321>
> Diffs
>
>    - 
> /src/org/waveprotocol/box/server/persistence/file/FileDeltaCollection.java
>    (1228763)
>
> View Diff <https://reviews.apache.org/r/3432/diff/>
>

Reply via email to