- Jul 28, 2017
-
-
Matej Urbančič authored
-
- Jul 02, 2017
-
-
Michael Catanzaro authored
It's wrong to acquire the session inhibitor when destination is created, because the download could fail before that point, causing the inhibitotr to be released before it has been acquired and triggering our assertions to ensure this does not happen. Instead, acquire the inhibitor immediately when creating the download. https://bugzilla.gnome.org/show_bug.cgi?id=778653
-
- Apr 20, 2017
-
-
We need to compare against the character, not the pointer which is done immediately to the left of this comparison.
-
- Apr 04, 2017
-
-
- Mar 19, 2017
-
-
Michael Catanzaro authored
-
- Mar 04, 2017
-
-
Michael Catanzaro authored
Don't call remove_from_destroy_list_cb, which modifies the destroy list, when already iterating through the list. https://bugzilla.gnome.org/show_bug.cgi?id=779180
-
Michael Catanzaro authored
Yeah this is really bad, but let's not make it fatal. I changed this in 3f76e6e5 but I'm not sure if it was intentional. It doesn't look like it, because I don't like leaving unreachable code after calling g_error(). I think I was probably just considering the change and forgot to turn it back to g_warning().
-
- Feb 24, 2017
-
-
Michael Catanzaro authored
Instead of having a longstanding transaction open at all times and scheduling commits that open a new transaction, just create transactions around history messages, so that each message forms its own atomic transaction. This is way simpler. I considered that we might not need transactions at all, but there are performance implications to removing transactions entirely.
-
Michael Catanzaro authored
-
- Feb 21, 2017
-
-
Michael Catanzaro authored
The design of the history service feels like one big footgun. I'm really not sure why a history thread is necessary at all, or why we have longstanding transactions (defeating the entire purpose of transactions) instead of just using autocommit, which I think would be sufficient for everything we do. This commit doesn't fix any of that. That's just a rant. This commit just fixes one theoretical race condition. Prepared statements lock the database and need to be finalized BEFORE commit. The current code only works if the prepared statement is finalized on the UI thread before the scheduled commit occurs on the history thread. Which is probably always, but let's not leave it to luck. I could see this leading to a small loss of the last bit of history when closing the browser. https://bugzilla.gnome.org/show_bug.cgi?id=778649
-
- Feb 19, 2017
-
-
Michael Catanzaro authored
Now that SQLite enforces read-only mode for us, bugs like this will be uncovered.... https://bugzilla.gnome.org/show_bug.cgi?id=778649
-
Michael Catanzaro authored
This started out as a project to fix the read-only service test I just added. Initializing two history service objects in a row was racy, because I needed the first history service to be initialized before creating the second one, but there was no way to ensure that. This was only an issue for this one test, though; real Epiphany browser mode of course only creates one history service, so I assumed it was not a big problem. Fix this first issue using a condition variable to ensure the GObject initialization doesn't complete until after the history service has actually created the SQLite database. In doing this, I discovered a second bug. The use of the condition variable altered the timing slightly, and caused the history filename property to not be set in time when entering the history service thread. In fact, it's kind of amazing that the history service ever worked at all, because there is absolutely nothing here to guarantee that the filename and read-only properties have been initialized prior to starting the history service thread. So the database filename could be NULL when opening the database, which is a great way to lose all your history. Also, it could also be in read-only mode here even if it is supposed to be read/write mode, which is going to cause failures after today's commits. Fix this by adding a constructed function and starting the history thread from there, instead of doing it in init. This means that the history thread will not be started until after properties have been set. Note that, while I could not reproduce this bug on my machine until after adding the condition variable to fix the first bug, that was just due to timing and luck; it was already broken before. https://bugzilla.gnome.org/show_bug.cgi?id=778649
-
Michael Catanzaro authored
The code does something different, and it's not complex enough to merit a comment anyway. https://bugzilla.gnome.org/show_bug.cgi?id=778649
-
Michael Catanzaro authored
Now that clear all is implemented by deleting the database file, there's no longer any need to schedule a commit here. https://bugzilla.gnome.org/show_bug.cgi?id=778649
-
Michael Catanzaro authored
There's no excuse for this.... https://bugzilla.gnome.org/show_bug.cgi?id=778649
-
Michael Catanzaro authored
Closing the connection is great, but not enough. We're leaking our wrapper object. https://bugzilla.gnome.org/show_bug.cgi?id=778649
-
Michael Catanzaro authored
We have assertions to ensure that several functions are only ever called on the history thread. But the first such assertion, at the top of run_history_service_thread, sometimes fails when running the tests. It is racy. Use a mutex to fix this. These assertions are actually executed at runtime for end users, so it's surprising that nobody has ever reported a bug about this. We also need to be sure to initialize the async queue before running the history service thread. The mutex is needed as a memory barrier here, so it's not possible to remove the mutex by removing the assertions except in debug mode, which is something I considered. https://bugzilla.gnome.org/show_bug.cgi?id=778649
-
Michael Catanzaro authored
If the search provider is running, all database transactions will fail because the search provider will take a write lock on the database. Ouch! This is worth a good string of profanities.... Notably, this causes opening the database to fail if you searched for anything in the shell overview in the minute prior to starting Epiphany. (One minute is our search provider timeout.) Then no history will ever get saved, ever. I think. Something like that. So, although our history service has read-only mode, it's enforced at the history service level, not the SQLite connection level. SQLite actually has a read-only mode, which we are not using, and which we need to use in the search provider if we want to have any chance of reliably saving history. Accordingly, give EphySQLiteConnection a mode property, to indicate whether it is in writable mode or read-only mode. Teach all callers to set it properly. Use it, rather than a boolean, when creating the EphyHistoryService, since boolean parameters are hard to read at call sites. And actually put the underlying SQLite connection in read-only mode when set. Don't open transactions or ever attempt to rollback in read-only mode, because that doesn't make any sense. This should never have been happening due to the history service level read-only checks, but it should be enforced at the SQLite connection level now, too. Avoid initializing tables when opening the database in read-only mode. This is obviously writing to the database, and now that we really have a read-only SQLite connection it fails. As it should. SQLite connection creation will now fail in case the connection is read-only and the database does not yet exist; it will no longer be created anyway. So handle this case gracefully. It's fine for the history service to return nothing in this case. This has the small advantage that the history thread will quit immediately after it's created in this case, so it's not constantly running if there's no history in incognito mode anymore. To check for this condition, we expose the underlying SQLite error; previously, only the error message was exposed outside of EphySQLiteConnection. Exposing the error isn't really necessary or sufficient, though, since it's super generic and we have to check if the file actually exists on disk anyway. Test it. Ensure that a read/write history service functions properly if it's running at the same time as a read-only history service. Using two read/write services here fails very badly, but when one of the services is read-only it works fine. Also, remove the original read-only service test. It only ever tested that creating a read-only history service with an empty history database would succeed. And, as I just explained, that fails now. Lastly, stop running a second history service for the search provider. It needed its own once upon a time when the search provider did not run an EphyShell instance. That changed when we stopped using the default web context, because nothing works without EphyEmbedShell now, as all sorts of places need it to get the embed's web context. And since EphyEmbedShell runs its own history service, the search provider can just use that now instead of running its own separate one. https://bugzilla.gnome.org/show_bug.cgi?id=778649
-
Michael Catanzaro authored
These signals can run after the popover has been destroyed. We don't want that. Speculative fix for this critical: epiphany-Gtk-CRITICAL: gtk_widget_set_sensitive: assertion 'GTK_IS_WIDGET (widget)' failed https://bugzilla.gnome.org/show_bug.cgi?id=778659
-
Michael Catanzaro authored
This file is so careful to handle errors properly everywhere EXCEPT the point where actual SQLite commands are executed. The history database is pretty much totally broken right now; having error messages would be helpful thank you!
-
- Feb 14, 2017
-
-
Michael Catanzaro authored
-
Michael Catanzaro authored
Looks like I broke this in a refactoring, probably adc6c404. Snapshots loaded for the overview are almost always available in cache before a webpage is visited, so those pages would never get updated. We need to update stale snapshots in ephy_snapshot_service_get_snapshot_path_async() to avoid this. It *could* still happen that snapshot requests are scheduled multiple times in a row, but it's unlikely and harmless.
-
- Feb 03, 2017
-
-
Michael Catanzaro authored
-
This adds a few more cases to the escaping done when converting an AdBlock non-regepx "simple pattern" from a rule into a GRegex. This patch does the following: - Adds escaping to some of the regexp metacharacters which were not being handled: (){}+.|\ - Adds support for using a vertical bar at the end of a pattern to anchor the match at the end. - Adds support for using ^ to match a "separator character" (a non-letter, non-number, or one of _-.%). This also adds as much comment lines as code, which in this particular case is probably a good thing, so reading the code in the future does not need checking each case against the GRegex documentation. https://bugzilla.gnome.org/show_bug.cgi?id=777714
-
Michael Catanzaro authored
This is ephy *profile* migrator. It runs on a per-profile basis. i.e. each web app runs migrators separately. So this migration step could run once for a profile dir, then again far in the future when an old web app is opened. But passwords are global state, not stored in the profile dir, and we want to run this migration only once. This is tricky to fix, but it's easier if we relax the constraint to "never run this migrator if it has been run already for the default profile dir." That's because we don't really care if a couple web app passwords get converted from insecure to secure, which is not a big problem and indicates the user probably never uses Epiphany except for web apps anyway. We just don't want all the user's passwords to get converted mysteriously because he happens to open a web app. So check the migration version for the default profile dir and abort if this migrator has already run there. This way we avoid adding a new flag fil...
-
Michael Catanzaro authored
I've mishandled this issue pretty badly. Incredibly, my previous patch, which was intended to ensure we always normalize URIs to security origins when working with form auth data, only fixed use of the form auth data cache. It didn't actually fix any use of the secret service itself. Fix that. This commit notably removes support for mailman passwords, which is making the code way too complicated and conflicts with the goal of storing only security origins and not full URIs in the secret service. Note: this normalization is way better than what we were doing before. In particular, it incidentally fixes odd bugs like the URI framgment, even the empty fragment #, being sufficient to trick our password manager into storing separate passwords, so this should also make the password filling significantly more reliable than it used to be. (Unless you need per-URI passwords without a username, i.e. mailman passwords, in which case you're just out of luck, sorry!) https://bugzilla.gnome.org/show_bug.cgi?id=752738
-
- Feb 02, 2017
-
-
Michael Catanzaro authored
Using just host is not sufficient, we need to have protocol and port as well for matching based on security origin to work properly. Unfortunately the existing code here was full of subtle errors: the parameters named "uri" were actually passed hostnames from the web extension, and not URIs at all. The code only worked as long as that assumption held, but I broke it because I expected the URI parameters to actually contain URIs. So fix this. Really pass URIs and not hostnames, and properly convert them to security origins. Thanks to Hussam for reporting this bug so quickly after it was introduced. (As well as lots of other bugs in the past that I've rarely credited him for in commit messages.) https://bugzilla.gnome.org/show_bug.cgi?id=752738
-
- Feb 01, 2017
-
-
Michael Catanzaro authored
This reverts commit 60097baf. Seems to be causing problems, so let's not do this in gnome-3-22
-
Michael Catanzaro authored
All previously-saved passwords will now only be available to https:// origins. Users will have to manually enter their passwords once again in order to save them separately for an insecure origin. https://bugzilla.gnome.org/show_bug.cgi?id=752738
-
Michael Catanzaro authored
This prevents an active MITM attacker from enumerating all your saved passwords. The attacker will now only be able to access passwords saved on http:// sites. That's by design, though; users are now warned when focusing insecure password forms and should think twice before saving such passwords. Unfortunately this does introduce a migration issue, in that no previously-saved passwords will be available on https:// websites anymore, and all previously-saved passwords will still be enumerable by attackers. I'm not sure how to handle migration. We might be able to handle it nicely by using the history service to guess whether a password should be migrated from http:// to https://, but that is not a simple project. https://bugzilla.gnome.org/show_bug.cgi?id=752738
-
Michael Catanzaro authored
-
- Jan 30, 2017
-
-
Michael Catanzaro authored
This was a tough one. It's a GArray rather than a GPtrArray, which led me on a wild goose chase trying to set a clear function for the array... the clear function is not allowed to actually free memory, since GArray is not designed for holding pointers. This code should probably be refactored further. https://bugzilla.gnome.org/show_bug.cgi?id=682723
-
- Jan 26, 2017
-
-
Michael Catanzaro authored
-
- Jan 17, 2017
-
-
Michael Catanzaro authored
Got to do this because I accidentally broke the last overview item in 3.22.4 :(
-
- Jan 16, 2017
-
-
Michael Catanzaro authored
-
Michael Catanzaro authored
-
- Jan 14, 2017
-
-
Michael Catanzaro authored
The current info bar management code improperly assumes that the user will always close the info bar before closing its associated web view. If the user closes the web view first, then we leak the FormAuthData or PermissionRequestData struct in EphyWebView. In the case of PermissionRequestData, that notably contains an unresolved WebKitPermissionRequest. Additionally, for form auth data requests, the hash table entry for the outstanding request in the web extension is leaked because the web extension never receives the expected form auth data request response. Resolve this by tracking the destruction of the info bars with a weak reference.
-
Michael Catanzaro authored
-
Michael Catanzaro authored
The text is black, so give it a light background.
-