lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael McCandless (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LUCENE-3738) Be consistent about negative vInt/vLong
Date Sat, 17 Mar 2012 13:29:37 GMT

    [ https://issues.apache.org/jira/browse/LUCENE-3738?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231962#comment-13231962
] 

Michael McCandless commented on LUCENE-3738:
--------------------------------------------

bq. The check is only ommitted in the unrolled loop, the for-loop still contains the check.

I'm confused... I don't see how/where BufferedIndexInput.readVLong is
checking for negative result now...?  Are you proposing adding an if
into that method?  That's what I don't want to do... eg, readVLong is
called 3 times per term we decode (Lucene40 codec); it's a very low
level API... other codecs may very well call it more often.  I don't
think we should add an if inside BII.readVLong.

Or.... maybe you are saying you just want the unrolled code to handle
the negative vLong case (ie, unroll the currently missing 10th cycle),
and not add an if to BufferedIndexInput.readVLong?  And then "for
free" we can add a real if (not assert) if that 10th cycle is hit?
(ie, if we get to that 10th byte, throw an exception).  I think that
makes sense!

bq. there are other asserts in the index readiung code at places completely outside any loops,
executed only once when index is opened. 

+1 to make those real checks, as long as the cost is vanishingly
small.

bq. which is also a security issue when you e.g. download indexes through network connections
and a man in the middle modifies the stream.

I don't think it's our job to protect against / detect that.

bq. Disk IO can produce wrong data.

True, but all bets are off if that happens: you're gonna get all sorts
of crazy exceptions out of Lucene.  We are not a filesystem.

                
> Be consistent about negative vInt/vLong
> ---------------------------------------
>
>                 Key: LUCENE-3738
>                 URL: https://issues.apache.org/jira/browse/LUCENE-3738
>             Project: Lucene - Java
>          Issue Type: Bug
>            Reporter: Michael McCandless
>            Assignee: Uwe Schindler
>             Fix For: 3.6, 4.0
>
>         Attachments: LUCENE-3738.patch, LUCENE-3738.patch, LUCENE-3738.patch
>
>
> Today, write/readVInt "allows" a negative int, in that it will encode and decode correctly,
just horribly inefficiently (5 bytes).
> However, read/writeVLong fails (trips an assert).
> I'd prefer that both vInt/vLong trip an assert if you ever try to write a negative number...
it's badly trappy today.  But, unfortunately, we sometimes rely on this... had we had this
assert in 'since the beginning' we could have avoided that.
> So, if we can't add that assert in today, I think we should at least fix readVLong to
handle negative longs... but then you quietly spend 9 bytes (even more trappy!).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message