Skip to content

Conversation

@flibustier
Copy link
Contributor

  • New output for wmap_sites -s
  • New option in wmap_sites for unicode tree output (wmap_sites -s [ids] (level) (unicode output true/false))
  • Filtering 404 in the tree output

You can see some of the informations in issue #8089

Verification

asciicast

…ites -s [ids] (true/false))

Filtering 404 in tree output
@wvu wvu added the feature label Mar 14, 2017
@wvu wvu self-assigned this Mar 14, 2017
end

#
# Print Tree structure. Still ugly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite!

if l == nil or l.empty?
l = 200
s = true
o = 'true'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not Boolean true?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it comes from the arg. You should be resolving the string to a Boolean.

l = l.to_i
s = false
# Add check if unicode parameters is the second one
if l == 'true' or l == 'false'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

|| is preferred over or.

Copy link
Contributor Author

@flibustier flibustier Mar 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry ! (Newbie in Ruby) Thanks, I will think about it the next time :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol, this was the only valid criticism. Nice work!

u = args.shift
l = args.shift
s = args.shift
o = args.shift
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the var change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume for "output?"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes exactly ! 's' was useless before so I recycle it !

end
end

o = (o == 'true')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you're resolving to Boolean down here. Can you move it earlier?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, an other option is this :

          o = args.shift
          o = (o == 'true')

          return unless u

          if l == nil or l.empty?
            l = 200
            o = true
          else
            # Add check if unicode parameters is the second one
            if l == 'true' || l == 'false'
              o = (l == 'true')
              l = 200
            else
              l = l.to_i
            end
          end

But it's involves two resolutions, not necessarily prettier…

@wvu
Copy link
Contributor

wvu commented Mar 14, 2017

This diff is difficult to read due to the gaps. Apologies if the comments don't read linearly. It seems you've already accounted for all my review points. Nice work!

@wvu
Copy link
Contributor

wvu commented Mar 14, 2017

So, do you think you can add Unicode detection like tree?

@busterb
Copy link
Contributor

busterb commented Mar 14, 2017

@wvu-r7 I believe our current approach is to assume everything but Windows is Unicode, or at least that's how the Meterpreter console does it.

    OptBool.new('EnableUnicodeEncoding', [true, "Automatically encode UTF-8 strings as hexadecimal", Rex::Compat.is_windows]),

Another approach would be to check the LANG environment variable contains UTF-8

@busterb
Copy link
Contributor

busterb commented Mar 14, 2017

If you come up with a common method we can share between the two locations, I'll bet there are some other areas we could start fixing too. Maybe we should add Rex::Compat.output_unicode or something.

wvu added a commit to wvu/metasploit-framework that referenced this pull request Mar 17, 2017
@wvu wvu merged commit 32edeb9 into rapid7:master Mar 17, 2017
@wvu
Copy link
Contributor

wvu commented Mar 17, 2017

Thanks, great work!

@flibustier
Copy link
Contributor Author

Thanks !! :)

@wvu
Copy link
Contributor

wvu commented Mar 22, 2017

Release Notes

Visual Improvements were made to the Unicode tree produced by WMAP plugin's wmap_sites -s command.

@tdoan-r7 tdoan-r7 added the rn-enhancement release notes enhancement label Mar 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature rn-enhancement release notes enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants