Repository: scrapy/scrapy

Issue:
Scrapy ignores proxy credentials when using "proxy" meta key
Code `yield Request(link, meta={'proxy': 'http://user:password@ip:port’})` ignores user:password. 
Problem is solved by using header "Proxy-Authorization" with base64, but it is better to implement it inside Scrapy.

Discussion:
elacuesta: I fixed something that was making the build fail, please check it again @redapple 
redapple: Regarding tests, I believe you can adapt https://github.com/scrapy/scrapy/blob/master/tests/test_downloadermiddleware_httpproxy.py#L50 and the subsequent ones (i.e. instead of setting proxy as an envvar, set `"proxy"` meta on the request and check Proxy-Auth header value)
elacuesta: Oh right, I was thinking about actually performing the requests, but I see now that checking the headers should be enough. Thanks Paul, I'll add tests similar to those 👍 
codecov-io: # [Codecov](https://codecov.io/gh/scrapy/scrapy/pull/2530?src=pr&el=h1) Report
> Merging [#2530](https://codecov.io/gh/scrapy/scrapy/pull/2530?src=pr&el=desc) into [master](https://codecov.io/gh/scrapy/scrapy/commit/3b8e6d4d820bf69ed3ad2e4d57eb6ca386323a5f?src=pr&el=desc) will **increase** coverage by `-0.02%`.


```diff
@@            Coverage Diff             @@
##           master    #2530      +/-   ##
==========================================
- Coverage   83.49%   83.48%   -0.02%     
==========================================
  Files         161      161              
  Lines        8787     8792       +5     
  Branches     1289     1290       +1     
==========================================
+ Hits         7337     7340       +3     
- Misses       1203     1204       +1     
- Partials      247      248       +1
```


| [Impacted Files](https://codecov.io/gh/scrapy/scrapy/pull/2530?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [scrapy/downloadermiddlewares/httpproxy.py](https://codecov.io/gh/scrapy/scrapy/compare/3b8e6d4d820bf69ed3ad2e4d57eb6ca386323a5f...90191a8a0fc87c37b03024baf04e3a9ef1b1d570?src=pr&el=tree#diff-c2NyYXB5L2Rvd25sb2FkZXJtaWRkbGV3YXJlcy9odHRwcHJveHkucHk=) | `95.91% <71.42%> (-4.09%)` | :x: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/scrapy/scrapy/pull/2530?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/scrapy/scrapy/pull/2530?src=pr&el=footer). Last update [3b8e6d4...37d781f](https://codecov.io/gh/scrapy/scrapy/compare/3b8e6d4d820bf69ed3ad2e4d57eb6ca386323a5f...37d781fdf43112e0e74d4fa0d43e2515a51ba3fd?el=footer&src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
elacuesta: Hi @redapple, I included some tests. I still have one doubt though. Should scrapy skip proxies based on `no_proxy` env var even for proxies set on `request.meta`? I think users would expect that explicitly setting a proxy in `meta` would override `no_proxy`, but I would like your opinion on this. Thank you!
redapple: I would say that a valid `"proxy"` in `Request.meta` takes precedence over env vars.
Documentation probably needs updating/clarifying regarding this.
elacuesta: Thanks, I added a few lines to the docs explaining that, please check. Another question, if you don't mind: [this code](https://github.com/scrapy/scrapy/blob/master/scrapy/downloadermiddlewares/httpproxy.py#L23-L24) raises `NotConfigured` if there are no proxies set on `http_proxy`/`https_proxy` env vars, effectively disabling the middleware even for requests with `meta['proxy']`. Should we change that?
redapple: Oh, nice catch. Indeed, checking "proxy" meta would require to have the middleware enabled all the time. I don't mind as long as the "no-pxoy set -> skip" case is fast enough.
redapple: Another option could be to have an extra setting like `HTTPPROXY_CHECK_REQUEST_META` of some sort, but I am not sure I like it.
elacuesta: ```
elif not self.proxies:
    return
``` 
after `if 'proxy' in request.meta` in the middleware's `process_request` would skip the processing after only two simple checks, I don't think that would have much performance penalty, what do you think?
redapple: `elif not self.proxies` would leave early enough, indeed.
elacuesta: What about a setting (like `HTTPPROXY_ENABLED`), allowing to completely disable the middleware? Not sure what should be the default value, though.

Patch:
diff --git a/.bumpversion.cfg b/.bumpversion.cfg
index b95e0bad5..0a8a71e8e 100644
--- a/.bumpversion.cfg
+++ b/.bumpversion.cfg
@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 1.3.2
+current_version = 1.3.1
 commit = True
 tag = True
 tag_name = {new_version}
diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst
index 3dc5ad2ed..8e14d1b7c 100644
--- a/docs/intro/tutorial.rst
+++ b/docs/intro/tutorial.rst
@@ -225,7 +225,7 @@ You will see something like::
     [s]   shelp()           Shell help (print this help)
     [s]   fetch(req_or_url) Fetch request (or URL) and update local objects
     [s]   view(response)    View response in a browser
-    >>>
+    >>> 
 
 Using the shell, you can try selecting elements using `CSS`_ with the response
 object::
@@ -423,7 +423,7 @@ in the callback, as you can see below::
             for quote in response.css('div.quote'):
                 yield {
                     'text': quote.css('span.text::text').extract_first(),
-                    'author': quote.css('small.author::text').extract_first(),
+                    'author': quote.css('span small::text').extract_first(),
                     'tags': quote.css('div.tags a.tag::text').extract(),
                 }
 
@@ -522,7 +522,7 @@ page, extracting data from it::
             for quote in response.css('div.quote'):
                 yield {
                     'text': quote.css('span.text::text').extract_first(),
-                    'author': quote.css('small.author::text').extract_first(),
+                    'author': quote.css('span small::text').extract_first(),
                     'tags': quote.css('div.tags a.tag::text').extract(),
                 }
 
@@ -568,7 +568,7 @@ this time for scraping author information::
 
         def parse(self, response):
             # follow links to author pages
-            for href in response.css('.author + a::attr(href)').extract():
+            for href in response.css('.author+a::attr(href)').extract():
                 yield scrapy.Request(response.urljoin(href),
                                      callback=self.parse_author)
 
@@ -624,7 +624,7 @@ option when running them::
     scrapy crawl quotes -o quotes-humor.json -a tag=humor
 
 These arguments are passed to the Spider's ``__init__`` method and become
-spider attributes by default.
+spider attributes by default.  
 
 In this example, the value provided for the ``tag`` argument will be available
 via ``self.tag``. You can use this to make your spider fetch only quotes
@@ -647,7 +647,7 @@ with a specific tag, building the URL based on the argument::
             for quote in response.css('div.quote'):
                 yield {
                     'text': quote.css('span.text::text').extract_first(),
-                    'author': quote.css('small.author::text').extract_first(),
+                    'author': quote.css('span small a::text').extract_first(),
                 }
 
             next_page = response.css('li.next a::attr(href)').extract_first()
diff --git a/docs/news.rst b/docs/news.rst
index ff1e4ce03..3c1d24561 100644
--- a/docs/news.rst
+++ b/docs/news.rst
@@ -3,16 +3,6 @@
 Release notes
 =============
 
-Scrapy 1.3.2 (2017-02-13)
--------------------------
-
-Bug fixes
-~~~~~~~~~
-
-- Preserve crequest class when converting to/from dicts (utils.reqser) (:issue:`2510`).
-- Use consistent selectors for author field in tutorial (:issue:`2551`).
-- Fix TLS compatibility in Twisted 17+ (:issue:`2558`)
-
 Scrapy 1.3.1 (2017-02-08)
 -------------------------
 
diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst
index 1ca78ccc6..0ef3fb071 100644
--- a/docs/topics/downloader-middleware.rst
+++ b/docs/topics/downloader-middleware.rst
@@ -681,7 +681,9 @@ HttpProxyMiddleware
    * ``no_proxy``
 
    You can also set the meta key ``proxy`` per-request, to a value like
-   ``http://some_proxy_server:port``.
+   ``http://some_proxy_server:port`` or ``http://username:password@some_proxy_server:port``.
+   Keep in mind this value will take precedence over ``http_proxy``/``https_proxy``
+   environment variables, and it will also ignore ``no_proxy`` environment variable.
 
 .. _urllib: https://docs.python.org/2/library/urllib.html
 .. _urllib2: https://docs.python.org/2/library/urllib2.html
@@ -949,8 +951,16 @@ enable it for :ref:`broad crawls <topics-broad-crawls>`.
 HttpProxyMiddleware settings
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+.. setting:: HTTPPROXY_ENABLED
 .. setting:: HTTPPROXY_AUTH_ENCODING
 
+HTTPPROXY_ENABLED
+^^^^^^^^^^^^^^^^^
+
+Default: ``True``
+
+Whether or not to enable the :class:`HttpProxyMiddleware`.
+
 HTTPPROXY_AUTH_ENCODING
 ^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/scrapy/VERSION b/scrapy/VERSION
index 1892b9267..3a3cd8cc8 100644
--- a/scrapy/VERSION
+++ b/scrapy/VERSION
@@ -1,1 +1,1 @@
-1.3.2
+1.3.1
diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py
index 498e3d60f..955b7630c 100644
--- a/scrapy/core/downloader/tls.py
+++ b/scrapy/core/downloader/tls.py
@@ -1,8 +1,6 @@
 import logging
 from OpenSSL import SSL
 
-from scrapy import twisted_version
-
 
 logger = logging.getLogger(__name__)
 
@@ -20,17 +18,11 @@ openssl_methods = {
     METHOD_TLSv12: getattr(SSL, 'TLSv1_2_METHOD', 6),   # TLS 1.2 only
 }
 
-if twisted_version >= (14, 0, 0):
-    # ClientTLSOptions requires a recent-enough version of Twisted.
-    # Not having ScrapyClientTLSOptions should not matter for older
-    # Twisted versions because it is not used in the fallback
-    # ScrapyClientContextFactory.
+# ClientTLSOptions requires a recent-enough version of Twisted
+try:
 
     # taken from twisted/twisted/internet/_sslverify.py
-
     try:
-        # XXX: this try-except is not needed in Twisted 17.0.0+ because
-        # it requires pyOpenSSL 0.16+.
         from OpenSSL.SSL import SSL_CB_HANDSHAKE_DONE, SSL_CB_HANDSHAKE_START
     except ImportError:
         SSL_CB_HANDSHAKE_START = 0x10
@@ -38,17 +30,10 @@ if twisted_version >= (14, 0, 0):
 
     from twisted.internet.ssl import AcceptableCiphers
     from twisted.internet._sslverify import (ClientTLSOptions,
+                                             _maybeSetHostNameIndication,
                                              verifyHostname,
                                              VerificationError)
 
-    if twisted_version < (17, 0, 0):
-        from twisted.internet._sslverify import _maybeSetHostNameIndication
-        set_tlsext_host_name = _maybeSetHostNameIndication
-    else:
-        def set_tlsext_host_name(connection, hostNameBytes):
-            connection.set_tlsext_host_name(hostNameBytes)
-
-
     class ScrapyClientTLSOptions(ClientTLSOptions):
         """
         SSL Client connection creator ignoring certificate verification errors
@@ -61,7 +46,7 @@ if twisted_version >= (14, 0, 0):
 
         def _identityVerifyingInfoCallback(self, connection, where, ret):
             if where & SSL_CB_HANDSHAKE_START:
-                set_tlsext_host_name(connection, self._hostnameBytes)
+                _maybeSetHostNameIndication(connection, self._hostnameBytes)
             elif where & SSL_CB_HANDSHAKE_DONE:
                 try:
                     verifyHostname(connection, self._hostnameASCII)
@@ -77,3 +62,8 @@ if twisted_version >= (14, 0, 0):
                             self._hostnameASCII, repr(e)))
 
     DEFAULT_CIPHERS = AcceptableCiphers.fromOpenSSLCipherString('DEFAULT')
+
+except ImportError:
+    # ImportError should not matter for older Twisted versions
+    # as the above is not used in the fallback ScrapyClientContextFactory
+    pass
diff --git a/scrapy/downloadermiddlewares/httpproxy.py b/scrapy/downloadermiddlewares/httpproxy.py
index 98c87aa9c..0d5320bf8 100644
--- a/scrapy/downloadermiddlewares/httpproxy.py
+++ b/scrapy/downloadermiddlewares/httpproxy.py
@@ -20,23 +20,25 @@ class HttpProxyMiddleware(object):
         for type, url in getproxies().items():
             self.proxies[type] = self._get_proxy(url, type)
 
-        if not self.proxies:
-            raise NotConfigured
-
     @classmethod
     def from_crawler(cls, crawler):
+        if not crawler.settings.getbool('HTTPPROXY_ENABLED'):
+            raise NotConfigured
         auth_encoding = crawler.settings.get('HTTPPROXY_AUTH_ENCODING')
         return cls(auth_encoding)
 
+    def _basic_auth_header(self, username, password):
+        user_pass = to_bytes(
+            '%s:%s' % (unquote(username), unquote(password)),
+            encoding=self.auth_encoding)
+        return base64.b64encode(user_pass).strip()
+
     def _get_proxy(self, url, orig_type):
         proxy_type, user, password, hostport = _parse_proxy(url)
         proxy_url = urlunparse((proxy_type or orig_type, hostport, '', '', '', ''))
 
         if user:
-            user_pass = to_bytes(
-                '%s:%s' % (unquote(user), unquote(password)),
-                encoding=self.auth_encoding)
-            creds = base64.b64encode(user_pass).strip()
+            creds = self._basic_auth_header(user, password)
         else:
             creds = None
 
@@ -45,6 +47,15 @@ class HttpProxyMiddleware(object):
     def process_request(self, request, spider):
         # ignore if proxy is already set
         if 'proxy' in request.meta:
+            if request.meta['proxy'] is None:
+                return
+            # extract credentials if present
+            creds, proxy_url = self._get_proxy(request.meta['proxy'], '')
+            request.meta['proxy'] = proxy_url
+            if creds and not request.headers.get('Proxy-Authorization'):
+                request.headers['Proxy-Authorization'] = b'Basic ' + creds
+            return
+        elif not self.proxies:
             return
 
         parsed = urlparse_cached(request)
diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py
index 24714a7a8..cb88bc2bf 100644
--- a/scrapy/settings/default_settings.py
+++ b/scrapy/settings/default_settings.py
@@ -174,6 +174,7 @@ HTTPCACHE_DBM_MODULE = 'anydbm' if six.PY2 else 'dbm'
 HTTPCACHE_POLICY = 'scrapy.extensions.httpcache.DummyPolicy'
 HTTPCACHE_GZIP = False
 
+HTTPPROXY_ENABLED = True
 HTTPPROXY_AUTH_ENCODING = 'latin-1'
 
 IMAGES_STORE_S3_ACL = 'private'
diff --git a/scrapy/utils/reqser.py b/scrapy/utils/reqser.py
index 2fceb0d94..7e1e99e48 100644
--- a/scrapy/utils/reqser.py
+++ b/scrapy/utils/reqser.py
@@ -5,7 +5,6 @@ import six
 
 from scrapy.http import Request
 from scrapy.utils.python import to_unicode, to_native_str
-from scrapy.utils.misc import load_object
 
 
 def request_to_dict(request, spider=None):
@@ -33,8 +32,6 @@ def request_to_dict(request, spider=None):
         'priority': request.priority,
         'dont_filter': request.dont_filter,
     }
-    if type(request) is not Request:
-        d['_class'] = request.__module__ + '.' + request.__class__.__name__
     return d
 
 
@@ -50,8 +47,7 @@ def request_from_dict(d, spider=None):
     eb = d['errback']
     if eb and spider:
         eb = _get_method(spider, eb)
-    request_cls = load_object(d['_class']) if '_class' in d else Request
-    return request_cls(
+    return Request(
         url=to_native_str(d['url']),
         callback=cb,
         errback=eb,
diff --git a/tox.ini b/tox.ini
index bbf50b733..0fdea11bb 100644
--- a/tox.ini
+++ b/tox.ini
@@ -21,16 +21,16 @@ passenv =
 commands =
     py.test --cov=scrapy --cov-report= {posargs:scrapy tests}
 
-[testenv:trusty]
+[testenv:precise]
 basepython = python2.7
 deps =
     pyOpenSSL==0.13
-    lxml==3.3.3
-    Twisted==13.2.0
-    boto==2.20.1
-    Pillow==2.3.0
+    lxml==2.3.2
+    Twisted==11.1.0
+    boto==2.2.2
+    Pillow<2.0
     cssselect==0.9.1
-    zope.interface==4.0.5
+    zope.interface==3.6.1
     -rtests/requirements.txt
 
 [testenv:jessie]


Test patch:
diff --git a/tests/test_downloadermiddleware_httpproxy.py b/tests/test_downloadermiddleware_httpproxy.py
index 2b26431a4..c77179ceb 100644
--- a/tests/test_downloadermiddleware_httpproxy.py
+++ b/tests/test_downloadermiddleware_httpproxy.py
@@ -1,11 +1,14 @@
 import os
 import sys
+from functools import partial
 from twisted.trial.unittest import TestCase, SkipTest
 
 from scrapy.downloadermiddlewares.httpproxy import HttpProxyMiddleware
 from scrapy.exceptions import NotConfigured
 from scrapy.http import Response, Request
 from scrapy.spiders import Spider
+from scrapy.crawler import Crawler
+from scrapy.settings import Settings
 
 spider = Spider('foo')
 
@@ -20,9 +23,10 @@ class TestDefaultHeadersMiddleware(TestCase):
     def tearDown(self):
         os.environ = self._oldenv
 
-    def test_no_proxies(self):
-        os.environ = {}
-        self.assertRaises(NotConfigured, HttpProxyMiddleware)
+    def test_not_enabled(self):
+        settings = Settings({'HTTPPROXY_ENABLED': False})
+        crawler = Crawler(spider, settings)
+        self.assertRaises(NotConfigured, partial(HttpProxyMiddleware.from_crawler, crawler))
 
     def test_no_enviroment_proxies(self):
         os.environ = {'dummy_proxy': 'reset_env_and_do_not_raise'}
@@ -47,6 +51,13 @@ class TestDefaultHeadersMiddleware(TestCase):
             self.assertEquals(req.url, url)
             self.assertEquals(req.meta.get('proxy'), proxy)
 
+    def test_proxy_precedence_meta(self):
+        os.environ['http_proxy'] = 'https://proxy.com'
+        mw = HttpProxyMiddleware()
+        req = Request('http://scrapytest.org', meta={'proxy': 'https://new.proxy:3128'})
+        assert mw.process_request(req, spider) is None
+        self.assertEquals(req.meta, {'proxy': 'https://new.proxy:3128'})
+
     def test_proxy_auth(self):
         os.environ['http_proxy'] = 'https://user:pass@proxy:3128'
         mw = HttpProxyMiddleware()
@@ -54,6 +65,11 @@ class TestDefaultHeadersMiddleware(TestCase):
         assert mw.process_request(req, spider) is None
         self.assertEquals(req.meta, {'proxy': 'https://proxy:3128'})
         self.assertEquals(req.headers.get('Proxy-Authorization'), b'Basic dXNlcjpwYXNz')
+        # proxy from request.meta
+        req = Request('http://scrapytest.org', meta={'proxy': 'https://username:password@proxy:3128'})
+        assert mw.process_request(req, spider) is None
+        self.assertEquals(req.meta, {'proxy': 'https://proxy:3128'})
+        self.assertEquals(req.headers.get('Proxy-Authorization'), b'Basic dXNlcm5hbWU6cGFzc3dvcmQ=')
 
     def test_proxy_auth_empty_passwd(self):
         os.environ['http_proxy'] = 'https://user:@proxy:3128'
@@ -62,6 +78,11 @@ class TestDefaultHeadersMiddleware(TestCase):
         assert mw.process_request(req, spider) is None
         self.assertEquals(req.meta, {'proxy': 'https://proxy:3128'})
         self.assertEquals(req.headers.get('Proxy-Authorization'), b'Basic dXNlcjo=')
+        # proxy from request.meta
+        req = Request('http://scrapytest.org', meta={'proxy': 'https://username:@proxy:3128'})
+        assert mw.process_request(req, spider) is None
+        self.assertEquals(req.meta, {'proxy': 'https://proxy:3128'})
+        self.assertEquals(req.headers.get('Proxy-Authorization'), b'Basic dXNlcm5hbWU6')
 
     def test_proxy_auth_encoding(self):
         # utf-8 encoding
@@ -72,6 +93,12 @@ class TestDefaultHeadersMiddleware(TestCase):
         self.assertEquals(req.meta, {'proxy': 'https://proxy:3128'})
         self.assertEquals(req.headers.get('Proxy-Authorization'), b'Basic bcOhbjpwYXNz')
 
+        # proxy from request.meta
+        req = Request('http://scrapytest.org', meta={'proxy': u'https://\u00FCser:pass@proxy:3128'})
+        assert mw.process_request(req, spider) is None
+        self.assertEquals(req.meta, {'proxy': 'https://proxy:3128'})
+        self.assertEquals(req.headers.get('Proxy-Authorization'), b'Basic w7xzZXI6cGFzcw==')
+
         # default latin-1 encoding
         mw = HttpProxyMiddleware(auth_encoding='latin-1')
         req = Request('http://scrapytest.org')
@@ -79,15 +106,21 @@ class TestDefaultHeadersMiddleware(TestCase):
         self.assertEquals(req.meta, {'proxy': 'https://proxy:3128'})
         self.assertEquals(req.headers.get('Proxy-Authorization'), b'Basic beFuOnBhc3M=')
 
+        # proxy from request.meta, latin-1 encoding
+        req = Request('http://scrapytest.org', meta={'proxy': u'https://\u00FCser:pass@proxy:3128'})
+        assert mw.process_request(req, spider) is None
+        self.assertEquals(req.meta, {'proxy': 'https://proxy:3128'})
+        self.assertEquals(req.headers.get('Proxy-Authorization'), b'Basic /HNlcjpwYXNz')
+
     def test_proxy_already_seted(self):
-        os.environ['http_proxy'] = http_proxy = 'https://proxy.for.http:3128'
+        os.environ['http_proxy'] = 'https://proxy.for.http:3128'
         mw = HttpProxyMiddleware()
         req = Request('http://noproxy.com', meta={'proxy': None})
         assert mw.process_request(req, spider) is None
         assert 'proxy' in req.meta and req.meta['proxy'] is None
 
     def test_no_proxy(self):
-        os.environ['http_proxy'] = http_proxy = 'https://proxy.for.http:3128'
+        os.environ['http_proxy'] = 'https://proxy.for.http:3128'
         mw = HttpProxyMiddleware()
 
         os.environ['no_proxy'] = '*'
@@ -104,3 +137,9 @@ class TestDefaultHeadersMiddleware(TestCase):
         req = Request('http://noproxy.com')
         assert mw.process_request(req, spider) is None
         assert 'proxy' not in req.meta
+
+        # proxy from meta['proxy'] takes precedence
+        os.environ['no_proxy'] = '*'
+        req = Request('http://noproxy.com', meta={'proxy': 'http://proxy.com'})
+        assert mw.process_request(req, spider) is None
+        self.assertEquals(req.meta, {'proxy': 'http://proxy.com'})
diff --git a/tests/test_utils_reqser.py b/tests/test_utils_reqser.py
index 5b889ab5d..a62f13e21 100644
--- a/tests/test_utils_reqser.py
+++ b/tests/test_utils_reqser.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 import unittest
 
-from scrapy.http import Request, FormRequest
+from scrapy.http import Request
 from scrapy.spiders import Spider
 from scrapy.utils.reqser import request_to_dict, request_from_dict
 
@@ -42,7 +42,6 @@ class RequestSerializationTest(unittest.TestCase):
         self._assert_same_request(request, request2)
 
     def _assert_same_request(self, r1, r2):
-        self.assertEqual(r1.__class__, r2.__class__)
         self.assertEqual(r1.url, r2.url)
         self.assertEqual(r1.callback, r2.callback)
         self.assertEqual(r1.errback, r2.errback)
@@ -55,12 +54,6 @@ class RequestSerializationTest(unittest.TestCase):
         self.assertEqual(r1.priority, r2.priority)
         self.assertEqual(r1.dont_filter, r2.dont_filter)
 
-    def test_request_class(self):
-        r = FormRequest("http://www.example.com")
-        self._assert_serializes_ok(r, spider=self.spider)
-        r = CustomRequest("http://www.example.com")
-        self._assert_serializes_ok(r, spider=self.spider)
-
     def test_callback_serialization(self):
         r = Request("http://www.example.com", callback=self.spider.parse_item,
                     errback=self.spider.handle_error)
@@ -84,7 +77,3 @@ class TestSpider(Spider):
 
     def handle_error(self, failure):
         pass
-
-
-class CustomRequest(Request):
-    pass