Merge pull request #154 from rubycdp/fix-149-authorize

Fix #149 authorize method accepts block

Author: route

README.md

@@ -346,7 +346,7 @@ browser.screenshot(path: "google.jpg") # => 30902
 # Save to Base64 the whole page not only viewport and reduce quality
 browser.screenshot(full: true, quality: 60) # "iVBORw0KGgoAAAANSUhEUgAABAAAAAMACAYAAAC6uhUNAAAAAXNSR0IArs4c6Q...
 # Save with specific background color
-browser.screenshot(background_color: Ferrum::RGBA.new(0, 0, 0, 0.0)) 
+browser.screenshot(background_color: Ferrum::RGBA.new(0, 0, 0, 0.0))
 ```
 
 #### pdf(\*\*options) : `String` | `Boolean`
@@ -484,22 +484,49 @@ end
 browser.go_to("https://google.com")
 ```
 
-#### authorize(\*\*options)
+#### authorize(\*\*options, &block)
 
-If site uses authorization you can provide credentials using this method.
+If site or proxy uses authorization you can provide credentials using this method.
 
 * options `Hash`
   * :type `Symbol` `:server` | `:proxy` site or proxy authorization
   * :user `String`
   * :password `String`
+* &block accepts authenticated request, which you must subsequently allow or deny, if you don't
+care about unwanted requests just call `request.continue`.
 
 ```ruby
-browser.network.authorize(user: "login", password: "pass")
+browser.network.authorize(user: "login", password: "pass") { |req| req.continue }
 browser.go_to("http://example.com/authenticated")
 puts browser.network.status # => 200
 puts browser.body # => Welcome, authenticated client
 ```
 
+Since Chrome implements authorize using request interception you must continue or abort authorized requests. If you
+already have code that uses interception you can use `authorize` without block, but if not you are obliged to pass
+block, so this is version doesn't pass block and can work just fine:
+
+```ruby
+browser = Ferrum::Browser.new
+browser.network.intercept
+browser.on(:request) do |request|
+  if request.resource_type == "Image"
+    request.abort
+  else
+    request.continue
+  end
+end
+
+browser.network.authorize(user: "login", password: "pass", type: :proxy)
+
+browser.go_to("https://google.com")
+
+```
+
+You used to call `authorize` method without block, but since it's implemented using request interception there could be
+a collision with another part of your code that also uses request interception, so that authorize allows the request
+while your code denies but it's too late. The block is mandatory now.
+
 
 ### Mouse
 

lib/ferrum/browser/client.rb

@@ -58,6 +58,10 @@ def on(event, &block)
         end
       end
 
+      def subscribed?(event)
+        [@interruptor, @subscriber].any? { |s| s.subscribed?(event) }
+      end
+
       def close
         @ws.close
         # Give a thread some time to handle a tail of messages

lib/ferrum/browser/subscriber.rb

@@ -21,6 +21,10 @@ def on(event, &block)
         true
       end
 
+      def subscribed?(event)
+        @on.key?(event)
+      end
+
       def call(message)
         method, params = message.values_at("method", "params")
         total = @on[method].size

lib/ferrum/network.rb

@@ -83,19 +83,21 @@ def intercept(pattern: "*", resource_type: nil)
       @page.command("Fetch.enable", handleAuthRequests: true, patterns: [pattern])
     end
 
-    def authorize(user:, password:, type: :server)
+    def authorize(user:, password:, type: :server, &block)
       unless AUTHORIZE_TYPE.include?(type)
         raise ArgumentError, ":type should be in #{AUTHORIZE_TYPE}"
       end
 
+      if !block_given? && !@page.subscribed?("Fetch.requestPaused")
+        raise ArgumentError, "Block is missing, call `authorize(...) { |r| r.continue } or subscribe to `on(:request)` events before calling it"
+      end
+
       @authorized_ids ||= {}
       @authorized_ids[type] ||= []
 
       intercept
 
-      @page.on(:request) do |request|
-        request.continue
-      end
+      @page.on(:request, &block)
 
       @page.on(:auth) do |request, index, total|
         if request.auth_challenge?(type)

lib/ferrum/node.rb

@@ -142,7 +142,7 @@ def find_position(x: nil, y: nil, position: :top)
 
       get_position(points, x, y, position)
     rescue Ferrum::BrowserError => e
-      return raise unless e.message&.include?('Could not compute content quads')
+      return raise unless e.message&.include?("Could not compute content quads")
 
       find_position_via_js
     end
@@ -151,8 +151,8 @@ def find_position(x: nil, y: nil, position: :top)
 
     def find_position_via_js
       [
-        evaluate('this.getBoundingClientRect().left + window.pageXOffset + (this.offsetWidth / 2)'), # x
-        evaluate('this.getBoundingClientRect().top + window.pageYOffset + (this.offsetHeight / 2)') # y
+        evaluate("this.getBoundingClientRect().left + window.pageXOffset + (this.offsetWidth / 2)"), # x
+        evaluate("this.getBoundingClientRect().top + window.pageYOffset + (this.offsetHeight / 2)") # y
       ]
     end
 

lib/ferrum/page.rb

@@ -180,6 +180,10 @@ def on(name, &block)
       end
     end
 
+    def subscribed?(event)
+      @client.subscribed?(event)
+    end
+
     private
 
     def subscribe

lib/ferrum/page/screenshot.rb

@@ -195,12 +195,12 @@ def with_background_color(color)
         if color
           raise ArgumentError, "Accept Ferrum::RGBA class only" unless color.is_a?(RGBA)
 
-          command('Emulation.setDefaultBackgroundColorOverride', color: color.to_h)
+          command("Emulation.setDefaultBackgroundColorOverride", color: color.to_h)
         end
 
         yield
       ensure
-        command('Emulation.setDefaultBackgroundColorOverride') if color
+        command("Emulation.setDefaultBackgroundColorOverride") if color
       end
     end
   end

spec/browser_spec.rb

@@ -97,8 +97,8 @@ module Ferrum
       expect(browser.viewport_size).to eq([200, 400])
     end
 
-    context 'fullscreen' do
-      shared_examples 'resize viewport by fullscreen' do
+    context "fullscreen" do
+      shared_examples "resize viewport by fullscreen" do
         it "allows the viewport to be resized by fullscreen" do
           expect(browser.viewport_size).to eq([1024, 768])
           browser.go_to(path)
@@ -107,12 +107,12 @@ module Ferrum
         end
       end
 
-      include_examples 'resize viewport by fullscreen' do
+      include_examples "resize viewport by fullscreen" do
         let(:path) { "/ferrum/custom_html_size" }
         let(:viewport_size) { [1280, 1024] }
       end
 
-      include_examples 'resize viewport by fullscreen' do
+      include_examples "resize viewport by fullscreen" do
         let(:path) { "/ferrum/custom_html_size_100%" }
         let(:viewport_size) { [1272, 1008] }
       end
@@ -564,7 +564,7 @@ module Ferrum
     it "does not run into content quads error" do
       browser.go_to("/ferrum/index")
 
-      allow_any_instance_of(Node).to receive(:get_content_quads).and_raise(Ferrum::BrowserError, 'message' => 'Could not compute content quads')
+      allow_any_instance_of(Node).to receive(:get_content_quads).and_raise(Ferrum::BrowserError, "message" => "Could not compute content quads")
 
       browser.at_xpath("//a[text() = 'JS redirect']").click
       expect(browser.body).to include("Hello world")

spec/network_spec.rb

@@ -201,6 +201,25 @@ module Ferrum
     end
 
     context "authentication support" do
+      it "raises error when authorize is without block" do
+        expect {
+          browser.network.authorize(user: "login", password: "pass")
+        }.to raise_exception(ArgumentError, "Block is missing, call `authorize(...) { |r| r.continue } or subscribe to `on(:request)` events before calling it")
+      end
+
+      it "raises no error when authorize is with block" do
+        expect {
+          browser.network.authorize(user: "login", password: "pass") { |r| r.continue }
+        }.not_to raise_error
+      end
+
+      it "raises no error when authorize is without block but subscribed to events" do
+        expect {
+          browser.on(:request) { |r| r.continue }
+          browser.network.authorize(user: "login", password: "pass")
+        }.not_to raise_error
+      end
+
       it "denies without credentials" do
         browser.go_to("/ferrum/basic_auth")
 
@@ -209,7 +228,9 @@ module Ferrum
       end
 
       it "allows with given credentials" do
-        browser.network.authorize(user: "login", password: "pass")
+        browser.network.authorize(user: "login", password: "pass") do |request|
+          request.continue
+        end
 
         browser.go_to("/ferrum/basic_auth")
 
@@ -218,7 +239,9 @@ module Ferrum
       end
 
       it "allows even overwriting headers" do
-        browser.network.authorize(user: "login", password: "pass")
+        browser.network.authorize(user: "login", password: "pass") do |request|
+          request.continue
+        end
         browser.headers.set("Cuprite" => "true")
 
         browser.go_to("/ferrum/basic_auth")
@@ -228,7 +251,9 @@ module Ferrum
       end
 
       it "denies with wrong credentials" do
-        browser.network.authorize(user: "user", password: "pass!")
+        browser.network.authorize(user: "user", password: "pass!") do |request|
+          request.continue
+        end
 
         browser.go_to("/ferrum/basic_auth")
 
@@ -237,7 +262,9 @@ module Ferrum
       end
 
       it "allows on POST request" do
-        browser.network.authorize(user: "login", password: "pass")
+        browser.network.authorize(user: "login", password: "pass") do |request|
+          request.continue
+        end
 
         browser.go_to("/ferrum/basic_auth")
         browser.at_css(%([type="submit"])).click

spec/rbga_spec.rb

@@ -4,43 +4,43 @@
 
 module Ferrum
   describe RGBA do
-    describe '#to_h' do
+    describe "#to_h" do
       it { expect(RGBA.new(0, 0, 0, 0.0).to_h).to eq({ r: 0, g: 0, b: 0, a: 0.0 }) }
       it { expect(RGBA.new(255, 255, 255, 1.0).to_h).to eq({ r: 255, g: 255, b: 255, a: 1.0 }) }
     end
 
-    it 'raises ArgumentError for not Float value of alpha' do
+    it "raises ArgumentError for not Float value of alpha" do
       expect {
         RGBA.new(0, 0, 0, 0)
       }.to raise_exception(
              ArgumentError,
-             'Wrong alpha value 0 should be Float between 0.0 (fully transparent) and 1.0 (fully opaque)')
+             "Wrong alpha value 0 should be Float between 0.0 (fully transparent) and 1.0 (fully opaque)")
     end
 
-    it 'raises ArgumentError wrong value of alpha' do
+    it "raises ArgumentError wrong value of alpha" do
       expect {
         RGBA.new(0, 0, 0, 2.0)
       }.to raise_exception(
              ArgumentError,
-             'Wrong alpha value 2.0 should be Float between 0.0 (fully transparent) and 1.0 (fully opaque)')
+             "Wrong alpha value 2.0 should be Float between 0.0 (fully transparent) and 1.0 (fully opaque)")
     end
 
-    it 'raises ArgumentError for wrong value of red' do
+    it "raises ArgumentError for wrong value of red" do
       expect {
         RGBA.new(-1, 0, 0, 0.0)
-      }.to raise_exception(ArgumentError, 'Wrong value of -1 should be Integer from 0 to 255')
+      }.to raise_exception(ArgumentError, "Wrong value of -1 should be Integer from 0 to 255")
     end
 
-    it 'raises ArgumentError for wrong value of green' do
+    it "raises ArgumentError for wrong value of green" do
       expect {
         RGBA.new(0, 256, 0, 0.0)
-      }.to raise_exception(ArgumentError, 'Wrong value of 256 should be Integer from 0 to 255')
+      }.to raise_exception(ArgumentError, "Wrong value of 256 should be Integer from 0 to 255")
     end
 
-    it 'raises ArgumentError for wrong value of blue' do
+    it "raises ArgumentError for wrong value of blue" do
       expect {
         RGBA.new(0, 0, 0.0, 0.0)
-      }.to raise_exception(ArgumentError, 'Wrong value of 0.0 should be Integer from 0 to 255')
+      }.to raise_exception(ArgumentError, "Wrong value of 0.0 should be Integer from 0 to 255")
     end
   end
 end