[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#719950: [RFR] templates://miniupnpd/{miniupnpd.templates}



On 08/22/2013 01:08 PM, Justin B Rye wrote:
> Christian PERRIER wrote:
>> Preamble: I'm not entirely happy with this review. I feel like the
>> whole debconf templates do no look very idiomatic but I found no way
>> to give them less French accent..:-)
> 
>> --- miniupnpd.old/debian/miniupnpd.templates	2013-08-17 07:53:13.557776725 +0200
>> +++ miniupnpd/debian/miniupnpd.templates	2013-08-22 09:29:43.789726277 +0200
>> @@ -2,24 +2,24 @@
>>  Type: boolean
>>  Default: false
>>  _Description: Start the MiniUPnP daemon?
> 
> Hang on, it's not asking if it should happen this once.  We usually
> phrase this as something like
> 
>    _Description: Start the MiniUPnP daemon at boot?

Which I don't like, because it's not only at boot time, but also right
after the package is installed.

>> + Please choose this option if you want to automatically start the MiniUPnP daemon at boot time.
> 
> Why does it default to false?

Because that's otherwise a security concern. If installed using the
non-interactive mode, then it may be possible that MiniUPNPd listens on
the WAN, which is just bad. So, by default, it's best to have it
disabled, and only activate when we are sure that the user has answered
properly to the Debconf questions.

>>  Template: miniupnpd/listen
>>  Type: string
>> +_Description: IP address to listen for UPnP queries on the local network:
> 
> It's the IP address to listen on⁁ on the local network, which is ugly!
> Maybe:
> 
>    _Description: Local address to listen on for UPnP queries:

Hum... I would like to insist hard that we want a LAN address here. If
by mistake, MiniUPNPd listens on the WAN IP, then there is security
consequences.

>> + The MiniUPnP daemon will listen for requests on the local network. Please
>>   enter the IP address it should listen on.
>>
>> Just avoid possessive articles.You know the rationale: "that might
>> not be "my" network.
> 
> Well, it's the LAN where I have superuser access on the router, so it
> probably is "my local network" if only in the sense of being local to
> me.  And cutting out possessives can often result in the text being
> so uninformative that it would be better just to throw out the whole
> phrase (this is often true for "the system").  But fortunately "the 
> local network" feels completely natural. 

What do you suggest then? Keep the sentence as it is right now?

>>  Template: miniupnpd/iface
>>  Type: string
>>  _Description: External WAN network interface where to open ports:
> 
> Ah, now that sounds a bit French

LOL! :)
Not very surprising as the text is from me, right?

>, and we don't need to worry about it
> going on and on this time:
> 
>    _Description: External WAN network interface to open ports on:

Thanks.

> I also proposed that it should have a paragraph summarising why users
> might or might not want to enable it.  If there are no such
> considerations, why bother making it configurable?  But the pros and
> cons aren't obvious to me.

I think this goes beyond the scope of a Debconf template.

>>  Description: daemon providing UPnP Internet Gateway Device (IGD) services
> 
> It's UPnP plus NAT-PMP now; and do we really need to mention IGD here?

I believe we do. It may help having relevant search results and is on
topic. For example, there's linux-igd as well (which is the reason why
miniupnpd is "mini").

> There's certainly no point explaining that "Internet Gateway Device"
> is abbreviated as "IGD" if we're going to do that all over again in 
> the long description.

Ok.

> More importantly, there's a major chunk of context completely missing
> from this package description!  If I see MiniUPnPd in the package
> repositories, think "that sounds useful", and install it on my laptop,
> it's not going to do me any good at all.
> 
>    Description: UPnP and NAT-PMP daemon for gateway routers
> 
> The man page also has a bit more useful explanation of how UPnP works,
> which I think we could afford to include in the long description.
> 
>>   MiniUPnPd is a small daemon providing UPnP Internet Gateway Device (IGD)
>> + services to the network. UPnP and NAT-PMP are used to improve Internet
> 
> This is one of the tricky cases where "the network" is hardly worth
> mentioning.

You could here replace it with "a local area network".

>>   connectivity for devices behind a NAT router. Any peer to peer network
>>   application such as games, IM, etc. can benefit from a NAT router supporting
> 
> That's a really oblique way of saying "don't bother installing it on
> your laptop".  Also, online games are rarely P2P, and the list could
> do with some sort of hint at Skype rather than just IM.
> 
>> + UPnP and/or NAT-PMP. For example, the latest generation Microsoft XBOX 360 and
>>   Sony Playstation 3 game machines use UPnP commands to enable the online play
>                                                                  ^^^
>>   with the XBOX Live service and the Playstation Network. It has been reported
>>   that MiniUPnPd is correctly working with the two consoles.
> 
> Surplus article.  More importantly, talking about the latest,
> modernest, most up-to-date things in a package description is a bad
> idea - this already has cobwebs.
> 
>> Maybe explain somewhere what UPnP means?
> 
> The trouble is, what it stands for is more of an advert than a
> meaningful name. NAT-PMP is better, but my suggested text below takes
> it for granted that if you're running Debian on your router then you
> already know terms like "NAT" and "LAN".
> 
>    Description: UPnP and NAT-PMP daemon for gateway routers
>     MiniUPnPd is a small daemon which can be installed on a NAT router to
>     provide UPnP Internet Gateway Device and Port Mapping Protocol services,
>     enabling clients on the LAN to ask for port redirections. It is
>     compatible with peer-to-peer software, messaging applications, and games
>     consoles that connect to online services (including XBOX Live and the
>     Playstation Network).

Nice! Thanks for your work and suggestions Justin.

Thomas


Reply to: