Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot call append_attribute in custom scrubber #173

Closed
unikitty37 opened this issue Nov 2, 2019 · 3 comments
Closed

Cannot call append_attribute in custom scrubber #173

unikitty37 opened this issue Nov 2, 2019 · 3 comments
Assignees

Comments

@unikitty37
Copy link

I'm trying to write a noreferrer scrubber, in the manner of the nofollow and noopener ones supplied.

The following code looks like it should work, but fails:

noreferrer = Loofah::Scrubber.new do |node|
  Loofah::Scrubber::append_attribute(node, 'rel', 'noreferrer') if (node.type == Nokogiri::XML::Node::ELEMENT_NODE) && (node.name == 'a')
end
Loofah.fragment(html).scrub!(noreferrer)

with NoMethodError: undefined method append_attribute' for Loofah::Scrubber:Class`.

I've tried just putting append_attribute — it doesn't look in Loofah at all, but instead uses the caller's context.

Placing this at the start of the helper file instead and replacing the call with scrub!(:noreferrer) also fails:

module Loofah
  module Scrubbers
    class NoReferrer < Scrubber
      def initialize
        @direction = :top_down
      end

      def scrub(node)
        return CONTINUE unless (node.type == Nokogiri::XML::Node::ELEMENT_NODE) && (node.name == 'a')
        append_attribute(node, 'rel', 'noreferrer')
        return STOP
      end
    end
  end
end

with Loofah::ScrubberNotFound: not a Scrubber or a scrubber name: :noreferrer — there doesn't seem to be anything in the documentation to tell me how to get Loofah to add the symbol for the new scrubber here.

Please could somebody tell me how to create a scrubber that uses append_attribute, short of copying the append_attribute definition into my own code?

Could I also gently suggest that, at times of frustration like this, seeing It's super-easy to write your own custom scrubbers for whatever document manipulation you need. in the README is not helpful — it reads like "everyone else has managed this — what are you, thick?"

@flavorjones
Copy link
Owner

Hi, thanks for asking this question and raising these issues. Clearly it's not as easy as I've aspired to make it, so I appreciate the feedback.

The append_attribute is a Scrubber instance method, not a class method, so that explains why you're getting this error. This method was originally intended to be use for internal implementations, but it feels like it's incorrect for this to be an instance method. It could be a class method (as it doesn't access or modify any Scrubber internal state) and so I'd like to fix that. Or maybe it should be a Nokogiri::XML::Node method, as it is very similar to Nokogiri::XML::Node#add_class.

Until I fix that, though you may want to consider modifying the node directly, or use a local method that reproduces append_attribute.

Sorry for the difficulties here.

@flavorjones flavorjones self-assigned this Dec 4, 2019
flavorjones added a commit to sparklemotion/nokogiri that referenced this issue Feb 25, 2020
- #kwattr_values
- #kwattr_add
- #kwattr_append
- #kwattr_remove

and rewrite CSS class convenience methods using their #kwattr analog.

related to flavorjones/loofah#173
@flavorjones
Copy link
Owner

Please note that I've started work adding this functionality to Nokogiri::XML::Node under this PR (still in progress): sparklemotion/nokogiri#2000

@flavorjones
Copy link
Owner

Just closing the loop here: Nokogiri v1.11.0.rc2 (when it comes out in a few days) will have a method Node#kwattr_add which you can use to add noreferrer.

If you're interested, I'd totally accept a pull request adding a noreferrer scrubber to Loofah. I'm available to assist if you'd like -- just poke me.

Thanks again for your patience and for taking the time to call my attention to this problem. Is there anything else I can do to help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants