Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.1.1
    • Fix Version/s: 4.2 Alpha1
    • Component/s: HttpCache
    • Labels:

      Description

      ResponseCachingPolicy currently uses integers for interpreting the size of Content-Length, as well internally.

      This causes issues in attempting to use the module for caching entities that are over 2GB in size, the module does not fail gracefully, but throws a NumberFormatException

      I have a patch that fixes this, by promoting the int -> long, which should allow for larger entities to be cached, it also updates the public facing API where possible, I don't think that the promotion should break compatibility massively

      The changes can also be seen here:
      https://github.com/GregBowyer/httpclient/commit/1197d3f94bd2eedcec32646cd6146748ca2e6fa1

        Activity

        Hide
        Greg Bowyer added a comment -

        Made the caching policy use longs instead of ints for content sizes

        Show
        Greg Bowyer added a comment - Made the caching policy use longs instead of ints for content sizes
        Hide
        Oleg Kalnichevski added a comment -

        Greg,
        There is certainly a way to fix the problem without breaking API compatibility at all, which would be preferred.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Greg, There is certainly a way to fix the problem without breaking API compatibility at all, which would be preferred. Oleg
        Hide
        Greg Bowyer added a comment -

        I might be dumb here, but the return type of CacheConfig.getMaxObjectSizeBytes currently returns an int, I cant see how to change that without breaking the API (a little)

        Everything else should be fine, but that method poses a problem

        Show
        Greg Bowyer added a comment - I might be dumb here, but the return type of CacheConfig.getMaxObjectSizeBytes currently returns an int, I cant see how to change that without breaking the API (a little) Everything else should be fine, but that method poses a problem
        Hide
        Oleg Kalnichevski added a comment -

        Change internal representation of maxObjectSizeBytes from int to long, deprecate CacheConfig.getMaxObjectSizeBytes make it return maxObjectSizeBytes value cast to int, add another method with a similar name (say CacheConfig#getMaxObjectSize) that returns maxObjectSizeBytes as long, do the same for setters, make sure HttpClient cache classes use new methods only.

        I think this should do the trick.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Change internal representation of maxObjectSizeBytes from int to long, deprecate CacheConfig.getMaxObjectSizeBytes make it return maxObjectSizeBytes value cast to int, add another method with a similar name (say CacheConfig#getMaxObjectSize) that returns maxObjectSizeBytes as long, do the same for setters, make sure HttpClient cache classes use new methods only. I think this should do the trick. Oleg
        Hide
        Greg Bowyer added a comment -

        Ah check, I thought that might add confusion as the API does not appear to have many deprecated methods. I will try to get the done by Monday

        If I cast getMaxObjectSizeBytes to an int, whhich is the desirable behaviour if maxObjectSizeBytes > Integer.MAX_VALUE

        • throw an exception
        • return (maxObjectSizeBytes >= Integer.MAX_VALUE) ? Integer.MAX_VALUE : (int) maxObjectSizeBytes
        • allow the thing to miscast and overflow into negative numbers

        I think the exception would be the preferred idea, but then that breaks the API at runtime :S

        Show
        Greg Bowyer added a comment - Ah check, I thought that might add confusion as the API does not appear to have many deprecated methods. I will try to get the done by Monday If I cast getMaxObjectSizeBytes to an int, whhich is the desirable behaviour if maxObjectSizeBytes > Integer.MAX_VALUE throw an exception return (maxObjectSizeBytes >= Integer.MAX_VALUE) ? Integer.MAX_VALUE : (int) maxObjectSizeBytes allow the thing to miscast and overflow into negative numbers I think the exception would be the preferred idea, but then that breaks the API at runtime :S
        Hide
        Oleg Kalnichevski added a comment -

        As far as I can tell the second option should be good enough.

        Oleg

        Show
        Oleg Kalnichevski added a comment - As far as I can tell the second option should be good enough. Oleg
        Hide
        Oleg Kalnichevski added a comment -

        Patch checked in with minor tweaks to preserve binary compatibility with 4.1

        Oleg

        Show
        Oleg Kalnichevski added a comment - Patch checked in with minor tweaks to preserve binary compatibility with 4.1 Oleg

          People

          • Assignee:
            Unassigned
            Reporter:
            Greg Bowyer
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development