After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 668865 - A couple of issues with SoupCache
A couple of issues with SoupCache
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: Misc
unspecified
Other Linux
: Normal normal
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2012-01-27 17:41 UTC by Sergio Villar
Modified: 2012-02-14 12:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Follow section 13.9 from specs (1.43 KB, patch)
2012-01-27 17:42 UTC, Sergio Villar
accepted-commit_now Details | Review
Do not generate a new request if it is not going to be conditional (3.38 KB, patch)
2012-01-27 17:42 UTC, Sergio Villar
accepted-commit_now Details | Review
Check cache control is not empty (830 bytes, patch)
2012-01-27 17:57 UTC, Sergio Villar
accepted-commit_now Details | Review

Description Sergio Villar 2012-01-27 17:41:52 UTC
I have identified two issues in the way SoupCache works:

1) it isn't respecting section 13.9 of the specs. Resources whose URL has a query and do not have expiration time provided by the server must not be cached.
2) if we try to generate a conditional request for a given resource, but the server didn't provided neither "Last-Modified" nor "ETag" then it makes no sense to generate an additional request because it won't be conditional (as they wouldn't have neither If-Modified-Since nor If-None-Match headers). The result is that, currently, we're generating 2 requests for those resources.
Comment 1 Sergio Villar 2012-01-27 17:42:29 UTC
Created attachment 206292 [details] [review]
Follow section 13.9 from specs
Comment 2 Sergio Villar 2012-01-27 17:42:59 UTC
Created attachment 206293 [details] [review]
Do not generate a new request if it is not going to be conditional
Comment 3 Sergio Villar 2012-01-27 17:57:53 UTC
Created attachment 206294 [details] [review]
Check cache control is not empty

Also this third minor fix. Some servers send an empty Cache-Control header, i.e, "", instead of not sending anything. That is the source of some criticals.
Comment 4 Dan Winship 2012-02-14 03:17:20 UTC
Comment on attachment 206293 [details] [review]
Do not generate a new request if it is not going to be conditional

>From 5716d9934a5a56513fdeda26e71949206fa88380 Mon Sep 17 00:00:00 2001
>From: Sergio Villar Senin <svillar@igalia.com>
>Date: Fri, 27 Jan 2012 16:24:56 +0100
>Subject: [PATCH 2/2] conditional request fixes
>
>---

needs a better commit message...

otherwise looks right
Comment 5 Dan Winship 2012-02-14 03:18:21 UTC
Comment on attachment 206294 [details] [review]
Check cache control is not empty

>+	if (cache_control && strlen (cache_control)) {

say "cache_control && *cache_control"; there's no need to spend the extra time measuring the whole length...
Comment 6 Sergio Villar 2012-02-14 12:39:11 UTC
Committed in master: 3c2341a1, e0a2af18 & a1d5f80e